-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The "Generic.WhiteSpace.DisallowSpaceIndent" doesn't handle mixed indents #547
Comments
If this will be fixed, then We can:
What's not right in above actions is that we're doing same actions for detecting wrong indentation as we'll be doing for fixing, but nobody have requested fixing at that point. This might affect sniff performance. |
This is intentional. When people use tabs, they are using them for general alignment and so that different developers can use different tab-width settings in their text editor. But if they switch to spaces at the end of the indent, they are using that for fine adjustments, like I use in the Squiz standard for arrays. If PHPCS detects So the sniff has to ignore them or else it will be generating errors based on the tab-width settings, which it should not be doing. The size of a tab shouldn't affect the way the file is reported on or the way the file is fixed or else the point of using tabs is gone. I know you've got a case where the spaces are a mistake and you want a tab, but I don't think the generic sniff should be fixing this, for the reasons above. |
This is already being detected, right?
OK. I guess I'll have to pay more attention during code reviews to spot these wrong indentation, that usually happens when code from space-indentation only project is copied to tab-indentation only project. Or maybe I can write my own sniff version to instantly report tab/space mixed usage in 1st whitespace on a line as an error (based on original token content before tabs are replaced with spaces)? I know for sure, that in my projects spaces are not used for fine tuning of indentation. |
Actually, maybe we can add a |
That would be great. I hope, that an extra IF for option checking won't slow down the sniff. I won't be able to use that option until next PHPCS release though. |
I recently ran into this problem too while setting up PHP_CodeSniffer for the first time.
While you're at it, would you also be able to incorporate a 'greedy' option? Something like #3 in @aik099's first reply. |
I'm not exactly sure what you are after. Is this for checking or fixing? Can you explain a bit more for me? |
For checking. I think it's already implemented in the fixer code. By greedy I mean that it converts as many spaces as it can into tabs according to the tab-width, but there may be some spaces left over for alignment adjustments. For example with a This is in contrast to a 'strict' option, which would see the mixture of tabs and spaces and output an error. The default option could be as it is now, accepting any spaces after tabs. |
I see what you mean now. Thanks for clarifying. Seems like a good option as well. |
I think it could be done by modifying line 104, although I don't know about the performance implications: if ($content[0] === ' ') { becomes if (strpos($content, ' ') !== false) { |
Sorry about the confusion, I just realised I had accidentally linked to pull number 3. As I think you've worked out, I meant point number 3 listed above. |
Also check this snippet please: public function run()
{
$long_1 = 'long1';
$long_2 = 'long2';
$s1 = 'short1'; // SHOULD NOT BE AN ERROR BUT IT IS
$s2 = 'short2'; // SHOULD NOT BE AN ERROR BUT IT IS
$s1 = 'short1'; // SHOULD BE AN ERROR BUT IT IS NOT
$s2 = 'short2'; // SHOULD BE AN ERROR BUT IT IS NOT
} |
Maybe it’s just a config issue which can be solved easily through rulesets, so I created a new issue for SmartTabs: #2480 |
When line is indented using
tab
+4 spaces
then this isn't reported as an error withtab-width
set to 4:The text was updated successfully, but these errors were encountered: