Comments (7)
I think we need a more experienced voice here, but as far as I know from many discussions here, the idea behind Fixer is that it has only processed file's context and the requirement is that the result is deterministic. I don't see how your fixer fits this 🤔.
In order to be able to sort methods the way they were defined in the parent class/interface, you would need to know that file's content, which couples Fixer with a runtime, because parent class/interface may be defined in the vendor directory. Currently you can run Fixer as a PHAR file or using a Docker image, and running analysis does not require vendors to be installed. There also can be magic autoloaders involved in the app to work, so Fixer should provide an API to configure it, which IMHO is a no-go (or maybe you want to require it in your fixer's constructor?).
Regardless of technical limitations, I believe such rule simply does not make sense. It does not matter how methods are ordered, the only thing that matters is that all required stuff is implemented (and this is not Fixer's job to check it). Such a rule could lead to a situation when you have constant changes in a file only because parent classes' / interfaces' authors changed the order of their methods. I also don't see how it would work when multiple interfaces are implemented by a class - I would rather see methods sorted alphabetically, than have them grouped by source. Not to mention traits, from which methods could be imported with an aliased name and fulfil the contract 🤷♂️. It's really, really complex stuff involving many little details and probably it's not something for single fixer.
I don't want to discourage you, but IMHO you should drop this idea 😉.
from php-cs-fixer.
I have already did that :) It uses reflection, which isn't idiomatic for PHP-CS-Fixer, but it does the job. I'm left to solve the problem with the order in which the fixers are applied so that the result of a run doesn't change after consecutive runs.
from php-cs-fixer.
Yeah, I saw the implementation, and it is vulnerable to everything I wrote. The result of the fixer is undeterministic because it depends on reflection+autoloading (the runtime), and external sources which may change the order of the methods causing different results. You do you, but for me it does not make sense to order methods like that (I don't see any value in it).
When it comes to changing the priority, I don't know if there's something preventing us from doing it, but I would like to know @keradus' opinion.
from php-cs-fixer.
I believe such rule simply does not make sense
That's why it is a custom fixer :)
You're absolutely correct about the fact, that it depends on the order of parents and causes a lot of changes when the author changes something. But that's the point of this fixer.
On my projects, this fixer makes sense since we have interfaces and many implementations of them. When methods are placed in different order during implementation, it sure doesn't make the review difficult, but it doesn't speed it up either. Also, this fixer helps to group implementations of different interfaces if there are several of them (even if implicitly, I plan another fixer for explicit grouping).
I agree that it's all just a matter of preferences. But you won't argue that the whole code style is a matter of preferences, will you? :)
On a higher level, I think the next major version of PHP-CS-Fixer should abolish the getPriority()
method and replace it with something like runAfter()
and runBefore()
, which would solve any such problems with standard and custom fixers.
from php-cs-fixer.
I agree that it's all just a matter of preferences. But you won't argue that the whole code style is a matter of preferences, will you? :)
Of course, without a doubt 🙂.
On a higher level, I think the next major version of PHP-CS-Fixer should abolish the
getPriority()
method and replace it with something likerunAfter()
andrunBefore()
, which would solve any such problems with standard and custom fixers.
I agree, this is something I pointed out (e.g. here and here) already. It shouldn't be needed to modify priorities in several fixers just to put a new one between them.
Anyway, we need to wait for the lead developer 😉.
from php-cs-fixer.
I believe @Wirone shared the context pretty well.
Contextless - as stated. Your Fixer violates that and for your custom needs it may be OK, but core repo may struggle to support that.
ordered_class_elements
- you suggest a rule that would be in same responsibility to existing rule, but with different sorting algo. I would suggest to not create new rule with priority dependention but replace/extend existing rule.
runAfter
- as discussed. PRs welcome.
Yet, my fixer should have priority higher than 65, but lower than 0
- regardless if this relation between built-in fixers or custom fixers is based on abstract numbers or linked via runAfter
, it won't solve the problem (you cannot declare graph that A runs after B, B runs after C, C runs after A
and expect to have it somehow resolved.
Changing the prio of rule - It's always risky and I am very careful here, because we assume there is no priority dependency , but there may simply be no declared, already discovered dependency. Having priority changed with no tests covering the need for that change will lead to someone coming in x months and changing it to another arbitrary number, effectively breaking the dependency order for you.
from php-cs-fixer.
Closing due to #7761 (comment)
from php-cs-fixer.
Related Issues (20)
- statement_indentation doesn't like multiline const
- `multiline_whitespace_before_semicolons` doesn't work with multiline const
- php_unit_test_class_requires_covers does not understand PHPUnit 10 attributes
- global_namespace_import ignores imports for symbols that exist in code but ain't detected HOT 5
- statement_indentation is indenting already correct line HOT 2
- How disable no_break_comment check? HOT 1
- Add double_quote option HOT 2
- trim_array_spaces fails to remove extra spaces before commas HOT 2
- no_trailing_whitespace_in_string new parameter to allow single white space if it exists HOT 6
- [InvalidArgumentException] Token at index xx is not the start of an alternative syntax block. HOT 1
- multiline_whitespace_before_semicolons behaves inconsistently with multline conditions
- Option's deprecation warning is misplaced in docs HOT 5
- `statement_indentation`: indents on multiline class properties HOT 3
- `switch` should be followed by a space HOT 5
- no_unused_imports does not remove some unused imports HOT 2
- Separat sorting for enum cases HOT 1
- "no-return" phpdoc not removed because superflous HOT 9
- Remove or add redundant `readonly` property modifier HOT 4
- ClassAttributesSeparationFixer - ['property' => 'none'] Add unwanted blank line when comment present
- Allow custom assertion methods in php_unit_test_case_static_method_calls
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from php-cs-fixer.