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

feat(lint): add support for config file and CLI flags for rules #11776

Merged
merged 20 commits into from
Sep 3, 2021

Conversation

bartlomieju
Copy link
Member

@bartlomieju bartlomieju commented Aug 19, 2021

This commit adds support for following flags in deno lint subcommand:

  • --config - allows to load configuration file and parses "lint" object
  • --rules-tags=<tags> - allows specifying which set of tagged rules should be run
  • --rules-include=<rules> - allow specifying which rules should be run
  • --rules-exclude=<rules> - allow specifying which rules should not be run

Ref #11686

cli/config_file.rs Outdated Show resolved Hide resolved
@bartlomieju bartlomieju added this to the 1.14.0 milestone Aug 19, 2021
cli/tools/lint.rs Outdated Show resolved Hide resolved
cli/config_file.rs Outdated Show resolved Hide resolved
cli/config_file.rs Outdated Show resolved Hide resolved
cli/tests/integration/lint_tests.rs Show resolved Hide resolved
cli/tests/testdata/lint/Deno.jsonc Show resolved Hide resolved
cli/tools/lint.rs Outdated Show resolved Hide resolved
cli/tools/lint.rs Show resolved Hide resolved
Copy link
Member

@lucacasonato lucacasonato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no flags for lint.rules.tags, lint.rules.include, and lint.rules.exclude.

cli/config_file.rs Outdated Show resolved Hide resolved
cli/config_file.rs Show resolved Hide resolved
@bartlomieju
Copy link
Member Author

There are no flags for lint.rules.tags, lint.rules.include, and lint.rules.exclude.

Correct. What's your suggestion here?

@lucacasonato
Copy link
Member

Correct. What's your suggestion here?

We agreed on no config file options that don't also have flags.

@bartlomieju
Copy link
Member Author

Correct. What's your suggestion here?

We agreed on no config file options that don't also have flags.

I can add the flags. Honestly having only files configuration is useless in my opinion.

cli/tools/lint.rs Outdated Show resolved Hide resolved
@bartlomieju bartlomieju changed the title feat(lint): add support for config file feat(lint): add support for config file and CLI flags for rules Aug 31, 2021
@bartlomieju
Copy link
Member Author

This PR is now feature complete, but I'm very unhappy about the code quality and intend to give it another pass to refactor it and make code better.

I also want to add some more integration tests to test all combinations of config/flags passed to "deno lint".

cli/config_file.rs Outdated Show resolved Hide resolved
cli/tools/lint.rs Show resolved Hide resolved
Copy link
Member

@dsherret dsherret left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@lucacasonato lucacasonato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Well done @bartlomieju.

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.

6 participants