BracesFixer by keradus · Pull Request #472 · PHP-CS-Fixer/PHP-CS-Fixer

keradus · 2014-08-23T17:09:48Z

Braces for structure blocks, fix #98

stof · 2014-08-27T00:43:13Z

Symfony/CS/Fixer/StructureBracesFixer.php

wrong indentation

stof · 2014-08-27T00:55:00Z

It looks like applying the new fixer on the project codebase changes lots of files, which looks wrong as I think the codebase already uses brances everywhere. It looks like there is a bug in your fixer

stof · 2014-08-27T00:56:35Z

Also, I think it will break for codebases using the following syntax:

if (2 > 1):
    echo 'This is always reached':
    echo 'More code';
endif;

the fixer should take such code into account to avoid breaking it

stof · 2014-08-27T08:08:23Z

Symfony/CS/Tokens.php

shouldn't you check for $index < count($this) to be safe against exceptions too ?

I do not see a case when this will break on valid php code. Could you provide one?

if you pass a $startBraceIndex which is not the opening brace, I think it might create weird issues

Actually whole project do not validate input parameters, so you can use almost any method in the way to blow up in your face.
It will be added, no harm

keradus · 2014-08-27T08:09:38Z

@stof

applying fixer on codebase - yes, it change a lot. it is not a bug, it just a fixer that is not finish yet. take a look at todo list - probably missing of correct multi-level intend change your code in bad way.
endif etc - argh, ugly syntax. actually consider between learning this fixer or ignore it (without codebreak) and creating new fixer to replace with {}

stof · 2014-08-27T08:10:11Z

Symfony/CS/Fixer/StructureBracesFixer.php

All this loop is the same than $tokens->findBracesBlockEnd but using parenthesis instead of braces. I suggest making findBracesBlockEnd able to handle both cases instead

good point. thanks ae5d0be

keradus · 2014-08-28T14:52:30Z

fix T_FOR token syntax

keradus · 2014-08-28T21:22:40Z

(just rebase on master)

keradus · 2014-08-29T12:33:14Z

Next to task done:

correct multi-level intend
make Travis pass

keradus · 2014-08-29T12:35:17Z

@stof
Running fixer on project itself do not change anything now. I think you may run it on your codebase again

Be aware that

if (2 > 1):
    echo 'This is always reached':
    echo 'More code';
endif;

syntax is not touched yet

stof · 2014-08-29T12:36:20Z

@keradus I don't have such codebase (the case where it is common to use such syntax is in templates, and I use Twig for that)

keradus · 2014-08-29T12:39:15Z

@stof OK. I think I will ignore this syntax, at least for now. Maybe some day.

The intresting thing is that the fixer do not only add missing braces, but also correct indent inside it ;)

stof · 2014-08-29T13:03:40Z

@keradus IMO, changing the if/endif syntax to the brace syntax is a job for a different fixer, as you might now want it globally

keradus · 2014-08-29T13:08:12Z

@stof Then agreed, this fixer should ignore that syntax.
Do you have time to create if ... endif -> if {} fixer ?

stof · 2014-09-08T20:39:01Z

👍

keradus · 2014-09-08T20:40:05Z

You are great @Nyholm ! Thanks !

Nyholm · 2014-09-08T21:31:26Z

Symfony/CS/Fixer/PSR2/BracesFixer.php

This line is not executed on HHVM (3.0.1). T_FINALLY is not defined. I don't know why. Sorry...

Finally was added in hhvm 3.2.0 btw.

Thanks @Nyholm !
Then I will just skip test if T_FINALLY is not defined.

T_FINALLY is defined on newer hhvm versions? If not, this is a bug, and should be reported to the hhvm team.

And travis uses hhvm 3.2.0, so how can this be happening?

Updated to HipHop VM 3.2.0 (rel) . Same error there.

This looks like a genuine bug in hhvm then.

I'll report it.

Thanks @Nyholm !

keradus · 2014-09-08T21:42:54Z

Wow, I see some ghost comments here ! :D

keradus · 2014-09-09T18:33:06Z

link facebook/hhvm#3703

keradus · 2014-09-09T18:41:10Z

merged

keradus mentioned this pull request Aug 23, 2014

Add curly braces when there is only one line inside if, for, etc. #98

Closed

keradus force-pushed the structure_braces branch 2 times, most recently from 7f88387 to 2bb29c1 Compare August 24, 2014 21:17

keradus force-pushed the master branch from 08ea2a4 to 7ee8b7f Compare August 25, 2014 21:03

keradus mentioned this pull request Aug 26, 2014

[minor] simplify Travis #474

Merged

stof reviewed Aug 27, 2014
View reviewed changes

keradus force-pushed the structure_braces branch from 62adc8f to deb41e9 Compare August 28, 2014 13:20

keradus mentioned this pull request Aug 28, 2014

Add Drupal code standards compatibility. #136

Closed

This was referenced Aug 28, 2014

Empty line before 'return' statement in "oneline" if block #341

Closed

Fixers that need rewrite to use Tokens #489

Closed

keradus force-pushed the structure_braces branch from 61520ec to 63fb018 Compare August 28, 2014 21:22

Nyholm reviewed Sep 8, 2014
View reviewed changes

BracesFixer - skip test if T_FINALLY is missing

07fe844

keradus force-pushed the structure_braces branch from 1e27854 to 07fe844 Compare September 8, 2014 21:40

BracesFixerTest - add link to hhvm issue

919faeb

keradus closed this in 32a4806 Sep 9, 2014

keradus deleted the structure_braces branch September 9, 2014 18:43

This was referenced Sep 24, 2014

PHP-CS-Fixer was wrong fix when "function" is end of comment line. #275

Closed

"do" keyword as (part of) argument's default value #191

Closed

Code Body Alignment Fixer #112

Closed

Space insertion inside string literal #305

Closed

keradus mentioned this pull request Oct 6, 2014

Inline control statement that's missing braces does NOT get fixed. #621

Closed

keradus mentioned this pull request Oct 17, 2014

fixDoWhile "fixes" all whiles after the first do #639

Closed

keradus removed their assignment Nov 6, 2014

keradus mentioned this pull request Dec 12, 2014

BracesFixer has too many responsibilities #823

Closed

keradus mentioned this pull request Sep 23, 2017

BinaryOperatorSpaces reworked #2573

Merged

keradus mentioned this pull request Feb 27, 2018

BracesFixer - handle comment for content outside of given block #3359

Closed

IllyaMoskvin mentioned this pull request Jul 21, 2019

SingleSpaceAfterConstructFixer - Introduction #4435

Merged

57 tasks

keradus mentioned this pull request Apr 2, 2023

minor: Deprecate BracesFixer #4885

Merged

2 tasks

Uh oh!

Conversation

keradus commented Aug 23, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stof commented Aug 27, 2014

Uh oh!

stof commented Aug 27, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

keradus commented Aug 27, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

keradus commented Aug 28, 2014

Uh oh!

keradus commented Aug 28, 2014

Uh oh!

keradus commented Aug 29, 2014

Uh oh!

keradus commented Aug 29, 2014

Uh oh!

stof commented Aug 29, 2014

Uh oh!

keradus commented Aug 29, 2014

Uh oh!

stof commented Aug 29, 2014

Uh oh!

keradus commented Aug 29, 2014

Uh oh!

stof commented Sep 8, 2014

Uh oh!

keradus commented Sep 8, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

keradus commented Sep 8, 2014

Uh oh!

keradus commented Sep 9, 2014

Uh oh!

keradus commented Sep 9, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants