Skip to content

Remove CSS/JS support (HUGE PR, reviews welcome!) #983

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

Merged
merged 9 commits into from
Apr 14, 2025

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Apr 14, 2025

Description

Remove all CSS-specific sniffs

Removes the following sniffs:

  • Generic.Debug.CSSLint
  • Squiz.CSS.ClassDefinitionClosingBraceSpace
  • Squiz.CSS.ClassDefinitionNameSpacing
  • Squiz.CSS.ClassDefinitionOpeningBraceSpace
  • Squiz.CSS.ColonSpacing
  • Squiz.CSS.ColourDefinition
  • Squiz.CSS.DisallowMultipleStyleDefinitions
  • Squiz.CSS.DuplicateClassDefinition
  • Squiz.CSS.DuplicateStyleDefinition
  • Squiz.CSS.EmptyClassDefinition
  • Squiz.CSS.EmptyStyleDefinition
  • Squiz.CSS.ForbiddenStyles
  • Squiz.CSS.Indentation
  • Squiz.CSS.LowercaseStyleDefinition
  • Squiz.CSS.MissingColon
  • Squiz.CSS.NamedColours
  • Squiz.CSS.Opacity
  • Squiz.CSS.SemicolonSpacing
  • Squiz.CSS.ShorthandSize

Ref:

Remove all JS-specific sniffs

Removes the following sniffs:

  • Generic.Debug.ClosureLinter
  • Generic.Debug.JSHint
  • Generic.Debug.ESLint
  • Squiz.Classes.DuplicateProperty
  • Squiz.Debug.JavaScriptLint
  • Squiz.Debug.JSLint
  • Squiz.Objects.DisallowObjectStringIndex
  • Squiz.Objects.ObjectMemberComma
  • Squiz.WhiteSpace.PropertyLabelSpacing

Includes removing two <exclude name="Generic.Debug"/> directives from tests for the Filter and Ruleset components.
These excludes were only introduced temporarily to bypass the deprecation notice for sniffing JS/CSS files with the Generic.Debug.* sniffs.

Ref:

Remove all JS/CSS specific code from mixed sniffs

Removes the JS/CSS support from the following sniffs:

  • Generic.Commenting.DocComment
  • Generic.Commenting.Fixme
  • Generic.Commenting.Todo
  • Generic.ControlStructures.InlineControlStructure
  • Generic.Files.EndFileNewline
  • Generic.Files.EndFileNoNewline
  • Generic.Files.LineEndings
  • Generic.Formatting.MultipleStatementAlignment
  • Generic.Formatting.SpaceAfterNot
  • Generic.PHP.LowerCaseConstant
  • Generic.Strings.UnnecessaryStringConcat
  • Generic.VersionControl.GitMergeConflict
  • Generic.WhiteSpace.DisallowTabIndent
  • Generic.WhiteSpace.DisallowSpaceIndent
  • Generic.WhiteSpace.IncrementDecrementSpacing
  • Generic.WhiteSpace.ScopeIndent
  • PEAR.ControlStructures.MultiLineCondition
  • PEAR.Functions.FunctionCallSignature
  • PEAR.Functions.FunctionDeclaration
  • Squiz.Commenting.DocCommentAlignment
  • Squiz.Commenting.FileComment
  • Squiz.Commenting.InlineComment
  • Squiz.Commenting.LongConditionClosingComment
  • Squiz.Commenting.PostStatementComment
  • Squiz.Formatting.OperatorBracket
  • Squiz.ControlStructures.ControlSignature
  • Squiz.ControlStructures.ForLoopDeclaration
  • Squiz.ControlStructures.SwitchDeclaration
  • Squiz.Functions.MultiLineFunctionDeclaration
  • Squiz.Operators.ComparisonOperatorUsage
  • Squiz.PHP.CommentedOutCode
  • Squiz.PHP.DisallowInlineIf
  • Squiz.PHP.DisallowSizeFunctionsInLoops
  • Squiz.WhiteSpace.ControlStructureSpacing
  • Squiz.WhiteSpace.FunctionClosingBraceSpace
  • Squiz.WhiteSpace.FunctionOpeningBraceSpace
  • Squiz.WhiteSpace.LogicalOperatorSpacing
  • Squiz.WhiteSpace.OperatorSpacing
  • Squiz.WhiteSpace.SemicolonSpacing
  • Squiz.WhiteSpace.SuperfluousWhitespace

Ref:

File::getDeclarationName(): remove support for JS functions

Ref:

Config: drop support for setting language via --extensions CLI arg

This has been implemented slightly differently from the original PHPCS 4.0 branch implementation.

  1. The array format of the Config::$extensions property has not been changed.
    The format is an associative array with the extensions as keys and the tokenizer type as the value.
    Originally, the format was changed to a list with the extensions as values.
    While information on the tokenizer type now no longer has any value as the only supported tokenizer is PHP, changing the array format could break integrations in unexpected ways, so I have decided against this.
  2. I have also elected to throw an error when the passed extensions contain the /tokenizer switch, but only if the tokenizer is set to something other than PHP.
    In the original implementation, no information was provided to the user and all extensions values would be accepted and expected to be acceptable, including values which contained the /tokenizer switch.

Note: users can still pass extensions for unsupported file types. This is no different than before, either in the previous implementation or in PHPCS 3.x. I mean, users could already pass html as an extension and it would be scanned as if it were PHP. Whether the results of such scans would be useful is doubtful, but that's the user's responsibility.
So if the user passes --extensions=php,js, as of PHPCS 4.0, javascript files will be scanned as if they were PHP.

Ref:

Ruleset: drop support for sniffs setting the $supportedTokenizers property

Ref:

Tokenizers: drop JS/CSS tokenizers and associated tokens

Ref:

File/utility methods: minor clean up

The T_DOC_COMMENT token is a PHP native token.

For PHP files, however, all T_DOC_COMMENT tokens are split up and re-tokenized via the PHP_CodeSniffer\Tokenizer\Comment class to more "sniffable" smaller tokens, so the T_DOC_COMMENT token should not exist in PHP files after the PHPCS tokenizer has finished processing the file.

By the looks of it, this may not have been the same for JS files. Then again, references to T_DOC_COMMENT - outside of the Tokenizer classes + the Tokens class - may also have been from long forgotten times before the Comment class existed.

Either way, now the JS and CSS tokenizers have been removed, it seems safe enough to remove references to T_DOC_COMMENT (outside of the Tokenizer context).

Miscellaneous other changes after dropping JS/CSS support

Closes squizlabs/PHP_CodeSniffer#2448

Related to #6

Suggested changelog entry

Changes:

  • The --extensions command line argument no longer accepts the tokenizer along with the extension.
    • Previously, you would check .module files as PHP files using --extensions=module/php.
    • Now, you use --extensions=module.

Removed:

  • Support for checking the coding standards of CSS and JS files has been removed.
    • Sniffs which are specifically aimed at CSS/JS files will no longer run.
    • All other sniffs will now treat all files under scan as PHP files.
    • The JS/CSS tokenizers and all tokens which were specifically for CSS/JS have also been removed.
  • The Generic.Debug.ClosureLinter sniff.
  • The Generic.Debug.CSSLint sniff.
  • The Generic.Debug.ESLint sniff.
  • The Generic.Debug.JSHint sniff.
  • The Squiz.Classes.DuplicateProperty sniff.
  • The Squiz.Debug.JavaScriptLint sniff.
  • The Squiz.Debug.JSLint sniff.
  • The Squiz.Objects.DisallowObjectStringIndex sniff.
  • The Squiz.Objects.ObjectMemberComment sniff.
  • The Squiz.WhiteSpace.PropertyLabelSpacing sniff.
  • The entire Squiz.CSS category, and all sniffs within.

gsherwood and others added 9 commits April 14, 2025 21:02
Removes the following sniffs:
- Generic.Debug.CSSLint
- Squiz.CSS.ClassDefinitionClosingBraceSpace
- Squiz.CSS.ClassDefinitionNameSpacing
- Squiz.CSS.ClassDefinitionOpeningBraceSpace
- Squiz.CSS.ColonSpacing
- Squiz.CSS.ColourDefinition
- Squiz.CSS.DisallowMultipleStyleDefinitions
- Squiz.CSS.DuplicateClassDefinition
- Squiz.CSS.DuplicateStyleDefinition
- Squiz.CSS.EmptyClassDefinition
- Squiz.CSS.EmptyStyleDefinition
- Squiz.CSS.ForbiddenStyles
- Squiz.CSS.Indentation
- Squiz.CSS.LowercaseStyleDefinition
- Squiz.CSS.MissingColon
- Squiz.CSS.NamedColours
- Squiz.CSS.Opacity
- Squiz.CSS.SemicolonSpacing
- Squiz.CSS.ShorthandSize

Ref:
* squizlabs/PHP_CodeSniffer 2448

Co-authored-by: jrfnl <[email protected]>
Removes the following sniffs:
- Generic.Debug.ClosureLinter
- Generic.Debug.JSHint
- Generic.Debug.ESLint
- Squiz.Classes.DuplicateProperty
- Squiz.Debug.JavaScriptLint
- Squiz.Debug.JSLint
- Squiz.Objects.DisallowObjectStringIndex
- Squiz.Objects.ObjectMemberComma
- Squiz.WhiteSpace.PropertyLabelSpacing

Includes removing two `<exclude name="Generic.Debug"/>` directives from tests for the Filter and Ruleset components.
These excludes were only introduced temporarily to bypass the deprecation notice for sniffing JS/CSS files with the `Generic.Debug.*` sniffs.

Ref:
* squizlabs/PHP_CodeSniffer 2448

Co-authored-by: jrfnl <[email protected]>
Removes the JS/CSS support from the following sniffs:
- Generic.Commenting.DocComment
- Generic.Commenting.Fixme
- Generic.Commenting.Todo
- Generic.ControlStructures.InlineControlStructure
- Generic.Files.EndFileNewline
- Generic.Files.EndFileNoNewline
- Generic.Files.LineEndings
- Generic.Formatting.MultipleStatementAlignment
- Generic.Formatting.SpaceAfterNot
- Generic.PHP.LowerCaseConstant
- Generic.Strings.UnnecessaryStringConcat
- Generic.VersionControl.GitMergeConflict
- Generic.WhiteSpace.DisallowTabIndent
- Generic.WhiteSpace.DisallowSpaceIndent
- Generic.WhiteSpace.IncrementDecrementSpacing
- Generic.WhiteSpace.ScopeIndent
- PEAR.ControlStructures.MultiLineCondition
- PEAR.Functions.FunctionCallSignature
- PEAR.Functions.FunctionDeclaration
- Squiz.Commenting.DocCommentAlignment
- Squiz.Commenting.FileComment
- Squiz.Commenting.InlineComment
- Squiz.Commenting.LongConditionClosingComment
- Squiz.Commenting.PostStatementComment
- Squiz.Formatting.OperatorBracket
- Squiz.ControlStructures.ControlSignature
- Squiz.ControlStructures.ForLoopDeclaration
- Squiz.ControlStructures.SwitchDeclaration
- Squiz.Functions.MultiLineFunctionDeclaration
- Squiz.Operators.ComparisonOperatorUsage
- Squiz.PHP.CommentedOutCode
- Squiz.PHP.DisallowInlineIf
- Squiz.PHP.DisallowSizeFunctionsInLoops
- Squiz.WhiteSpace.ControlStructureSpacing
- Squiz.WhiteSpace.FunctionClosingBraceSpace
- Squiz.WhiteSpace.FunctionOpeningBraceSpace
- Squiz.WhiteSpace.LogicalOperatorSpacing
- Squiz.WhiteSpace.OperatorSpacing
- Squiz.WhiteSpace.SemicolonSpacing
- Squiz.WhiteSpace.SuperfluousWhitespace

Ref:
* squizlabs/PHP_CodeSniffer 2448

Co-authored-by: jrfnl <[email protected]>
This has been implemented slightly differently from the original PHPCS 4.0 branch implementation.
1. The array format of the `Config::$extensions` property has not been changed.
    The format is an associative array with the extensions as keys and the tokenizer type as the value.
    Originally, the format was changed to a list with the extensions as values.
    While information on the tokenizer type now no longer has any value as the only supported tokenizer is PHP, changing the array format could break integrations in unexpected ways, so I have decided against this.
2. I have also elected to throw an error when the passed extensions contain the `/tokenizer` switch, but only if the tokenizer is set to something other than PHP.
    In the original implementation, no information was provided to the user and all extensions values would be accepted and expected to be acceptable, including values which contained the `/tokenizer` switch.

Note: users can still pass extensions for unsupported file types. This is no different than before, either in the previous implementation or in PHPCS 3.x. I mean, users could already pass `html` as an extension and it would be scanned as if it were PHP. Whether the results of such scans would be useful is doubtful, but that's the user's responsibility.
So if the user passes `--extensions=php,js`, as of PHPCS 4.0, javascript files will be scanned as if they were PHP.

Ref:
* squizlabs/PHP_CodeSniffer 2448

Co-authored-by: Greg Sherwood <[email protected]>
…roperty

Ref:
* squizlabs/PHP_CodeSniffer 2448

Co-authored-by: Greg Sherwood <[email protected]>
Ref:
* squizlabs/PHP_CodeSniffer 2448

Co-authored-by: Greg Sherwood <[email protected]>
The `T_DOC_COMMENT` token is a PHP native token.

For PHP files, however, all `T_DOC_COMMENT` tokens are split up and re-tokenized via the `PHP_CodeSniffer\Tokenizer\Comment` class to more "sniffable" smaller tokens, so the `T_DOC_COMMENT` token should not exist in PHP files after the PHPCS tokenizer has finished processing the file.

By the looks of it, this may not have been the same for JS files. Then again, references to `T_DOC_COMMENT` - outside of the Tokenizer classes + the `Tokens` class - may also have been from long forgotten times before the `Comment` class existed.

Either way, now the JS and CSS tokenizers have been removed, it seems safe enough to remove references to `T_DOC_COMMENT` (outside of the Tokenizer context).
Closes squizlabs/PHP_CodeSniffer 2448

Related to 6
@jrfnl jrfnl force-pushed the phpcs-4.0/feature/sq-2448-remove-support-js-css branch from e665d60 to 182f87a Compare April 14, 2025 19:02
@jrfnl
Copy link
Member Author

jrfnl commented Apr 14, 2025

Rebased without changes to hopefully get Coveralls to report 🤞🏻

@jrfnl
Copy link
Member Author

jrfnl commented Apr 14, 2025

Looks like Coveralls is struggling with the diff on this one. Manually setting the build to "Done" does give the Coverage difference, but doesn't let it report back to the GH API it seems.

The original build (from before I force pushed), finished after setting it to "Done", though was by that time marked as "Drifted".
All the same, it showed that the total coverage went up, not down with this PR, so let's just merge it.
image

@jrfnl jrfnl merged commit d91b9f2 into 4.x Apr 14, 2025
155 checks passed
@jrfnl jrfnl deleted the phpcs-4.0/feature/sq-2448-remove-support-js-css branch April 14, 2025 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants