Skip to content
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

feature/sniffs_system - twigcs #35

Open
wants to merge 53 commits into
base: master
Choose a base branch
from

Conversation

adrienrn
Copy link

@adrienrn adrienrn commented Nov 11, 2017

Here's twigcs - Twig Code Sniffer - which intend to be a "sniff engine" to check more thoroughly twig templates including whitespaces, trailing commas or missing parameters to functions/filters, etc.

If it sounds familiar, it's because it's heavily inspired on PHP_CodeSniffer.

I tried to not change anything to the current lint command. Everything should work as it was before.

Internals

There's two kind of sniffs:

  • Pre-parser: these are sniffs for formatting mainly, whitespace, blank lines, trailing comma, etc. They work using a custom tokenizer (a custom lexer), see "Why?" below.
  • Post-parser: these are "higher-order" sniffs, for functions, filters, check arguments value, usage, deprecation notices, etc. They work using Twig node visitors.

You define a set of rules (src/Twig/Lint/Ruleset.php) with options in a twigcs.yml file at the root of your twig project.

ruleset:
  -
    class: 'Asm89\Twig\Lint\Standards\Generic\Sniffs\DumpSniff'
    options:
      severity: 3
  -
    class: 'Asm89\Twig\Lint\Standards\Generic\Sniffs\WhitespaceBeforeAfterExpression'
    # no options means default severity eg. = 5

Once loaded, the Linter class (src/Twig/Lint/Linter.php) will process every requested files against that set of rules and produce a Report object (src/Twig/Lint/Report.php) with rule violations, context and statistics.

Running twigcs against the test fixtures files would produce:

./bin/twig-lint twigcs --working-dir tests/Asm89/Twig/Lint/Test/Fixtures/config ./tests/Asm89/Twig/Lint/Test/Fixtures/ --exclude hash_4.twig

image

Check the many PHPUnit tests I've made to dig further!

Todos

  • Sniffs via node visitors (post parser)
  • Pre parser sniffs via a custom Tokenizer (that handles whitespace, comment, and EOL tokens)
  • Base sniffs (Standards/Generic)
  • Linter main class
  • Load sniffs from config file
  • Error / Exception handling
  • Increase log-level
  • Stress test with 100~ templates and all sniffs (forking process?)
  • Add to twigcs.yml:
    • paths / files to process
    • exclude files / patterns
  • Allow to load sniffs from another directory (autoload, etc)
  • Handle deprecation notices from twig using set_error_handler. The StubbedCore makes it difficult but Allow 2 words tests tokens #24 should make it easier.
  • Severity option and filtering when displaying the report (or better, processing the sniffs).
  • Have a "Standards" feature, to make the definition easier. Now it's fine but the more sniff you'll have, the more twigcs.yml will be a clutter; and it would be nice to have a "standard" syntax like PSR-2 for twig, share by everyone.
  • Accept the template via stdin.
  • Documenting Tokenizer.php a bit more.
  • See if I can improve performance for the tokenizer. There's no performance problems yet, but (this article looks great!)[https://nikic.github.io/2011/10/23/Improving-lexing-performance-in-PHP.html].

Why?

In the end, I did it to integrate this to my CI process, using PHPstan or Codacy.

All of my many twig projects are maintained by many people (including freelance developers) and with the activity growing, I don't have time anymore to spend that much time reading pull requests 😔.

I created my own tokenizer / preprocessor out of the official lexer because it was not possible to detect space and all punctuations otherwise, see those issues:

Final notes!

If it's too much, I'll continue this on my fork or create a separate repository. :)

…DisallowTabIndentSniff + refactoring for tests
@asm89
Copy link
Owner

asm89 commented Nov 18, 2017

@adrienrn Woa, this is pretty cool! I need to properly sit down to go through this though. :)

@adrienrn
Copy link
Author

adrienrn commented Nov 28, 2017

Update:

Added some major things (more powerful config, external sniffs) and some minor ones (time / memory metrics, better output).

An example of the twigcs.yml of a project could be:

paths:
  - 'Resources/views'
exclude:
  - 'node_modules'
  - 'Resources/public'
ruleset:
  -
    class: 'Asm89\Twig\Lint\Standards\Generic\Sniffs\DeprecatedTemplateNotationSniff'
  -
    class: 'Asm89\Twig\Lint\Standards\Generic\Sniffs\DisallowCommentedCodeSniff'
  -
    class: 'Asm89\Twig\Lint\Standards\Generic\Sniffs\DisallowDumpSniff'
  -
    class: 'Asm89\Twig\Lint\Standards\Generic\Sniffs\DisallowIncludeTagSniff'
  -
    class: 'Asm89\Twig\Lint\Standards\Generic\Sniffs\DisallowTabIndentSniff'
  -
    class: 'Asm89\Twig\Lint\Standards\Generic\Sniffs\EnsureBlankAtEOFSniff'
  -
    class: 'Asm89\Twig\Lint\Standards\Generic\Sniffs\EnsureHashKeyQuotesSniff'
  -
    class: 'Asm89\Twig\Lint\Standards\Generic\Sniffs\EnsureHashSpacingSniff'
  -
    class: 'Asm89\Twig\Lint\Standards\Generic\Sniffs\EnsureHashTrailingCommaSniff'
  -
    class: 'Asm89\Twig\Lint\Standards\Generic\Sniffs\EnsureQuotesStyleSniff'
    options:
      style: 'TYPE_SINGLE_QUOTES'
  -
    class: 'Asm89\Twig\Lint\Standards\Generic\Sniffs\EnsureTranslationArgumentsSniff'
  -
    class: 'Asm89\Twig\Lint\Standards\Generic\Sniffs\EnsureWhitespaceExpressionSniff'

image

image

I also added the possibility to load custom sniffs; using the standardPaths configuration key.

You can add it to your twigcs.yml but it makes more sense to create a file ~/.twigcs/twigcs_global. Everything in that file will be merged with the one from your project.

ruleset: []
standardPaths:
  '\Acme\TwigCS':
    - '/home/vagrant/work/acme-twigcs/src'

Working on some proper "standard" next to easy things up.

Conflicts:
	src/Asm89/Twig/Lint/Command/LintCommand.php
	src/Asm89/Twig/Lint/StubbedEnvironment.php
	tests/Asm89/Twig/Lint/Test/StubbedEnvironmentTest.php
@seyfer
Copy link

seyfer commented Mar 7, 2019

@adrienrn see my comment here
#36

you better make your repo https://github.com/adrienrn/twig-lint
as a continuation :) then people can merge their PRs in your repo and you will maintain it, if you want

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants