stylelint icon indicating copy to clipboard operation
stylelint copied to clipboard

Add --rule and --no-stylelintrc CLI flags

Open shawnbot opened this issue 6 years ago • 11 comments

What is the problem you're trying to solve?

Stylelint autofixes turn out to be a really good way to refactor large codebases. However, it's painful having to modify your config by hand, and if you're extending an external config it's even trickier.

What solution would you like to see?

My proposal is a rule filter that could be enabled via the CLI, as in:

stylelint --rule-filter=some-rule --fix

or via the Node API:

const stylelint = require('stylelint')
stylelint.lint({
  files: '**/*.scss',
  fix: true,
  ruleFilter: 'some-rule'
})

Ideally, rules would be matched against the fully loaded config. It could be as simple as:

// anymatch allows for the same glob patterns as the "files" option
const anymatch = require('anymatch')
const {rules} = finalConfig
const {ruleFilter} = options
if (ruleFilter) {
  const filterRuleName = anymatch(ruleFilter)
  for (const ruleName of Object.keys(rules)) {
    if (!filterRuleName(ruleName)) {
      delete rules[ruleName]
    }
  }
}

Prior art

eslint has a --rule option, as mentioned in https://github.com/eslint/eslint/issues/6333. Generally I think parity between the two is good, so having a --rule option would be nice, too.

There's also a standalone tool called eslint-nibble that provides a nicer UX for tackling lots of rule violations. This is probably the route that I'll go if there isn't any interest in making these changes to stylelint directly. Either way, I'm happy to help with the implementation too.

shawnbot avatar Oct 07 '19 05:10 shawnbot

I don't understand the usefulness of this feature.

However, it's painful having to modify your config by hand

Isn't it the same as modifying CLI command?

hudochenkov avatar Oct 12 '19 21:10 hudochenkov

Isn't it the same as modifying CLI command?

What I'm proposing is that --rule would disable all the other rules so that you could focus on a single one. So if your config has dozens (if not hundreds) of rules, then the answer to your question is no.

One problem with doing it in a standalone tool is having to reimplement all of the same configuration loading logic that stylelint does. If that's just using the cosmiconfig API, then I guess it's not a huge deal. But I still think that giving people a way to focus on one or more rules — even just to spot-check a change they're making, particularly in big refactors — would be super useful.

shawnbot avatar Oct 14 '19 16:10 shawnbot

It will be confusing for people who use --rule in ESLint. There --rule specify a rule.

One problem with doing it in a standalone tool is having to reimplement all of the same configuration loading logic that stylelint does.

~~We have --print-config flag, which could be used to get config for a file. It prints into terminal. We don't have Node.js API for that.~~

I see you're using it in unofficial way :)

hudochenkov avatar Oct 27 '19 10:10 hudochenkov

It will be confusing for people who use --rule in ESLint. There --rule specify a rule.

Ah yeah, good point. 🤔 If you're still open to this idea, I'd be happy to hash out alternatives, such as:

  • --only, as in --only some-rule or --only some-rule,another-rule
  • --rules (plural) as in --rules some-rule,another-rule

My original proposal for --rule-filter is more explicit, though, so that might still be the best bet.

~We have --print-config flag, which could be used to get config for a file. It prints into terminal. We don't have Node.js API for that.~

I see you're using it in unofficial way :)

Yeah, it was the best way that I see to do it without spawning a stylelint --print-config CLI process and capturing stdout. 😬

shawnbot avatar Oct 30 '19 21:10 shawnbot

I see this feature as not something that would benefit majority of our users. Therefore, I think stylelint-only is a good solution. As it provides extra functionality for people who need it.

I'm curious what @stylelint/contributors think.

hudochenkov avatar Oct 30 '19 22:10 hudochenkov

Looking at the ESLInt --rule docs could stylelint do the same:

stylelint --rule 'quotes: [2, double]' --no-stylelintrc

That brings parity of ESLint to stylelint, also allows only a single rule to be used?

ntwb avatar Oct 30 '19 23:10 ntwb

stylelint-only looks very useful for the reasons you've stated here. I need to keep it in mind!

That being said I'm not sure it's core functionality for this project and my current opinion is leaning towards it remaining separate.

nosilleg avatar Oct 30 '19 23:10 nosilleg

That brings parity of ESLint to stylelint, also allows only a single rule to be used?

Looks like you can specify --rule multiple times to build up multiple rules

eslint --rule 'guard-for-in: 2' --rule 'brace-style: [2, 1tbs]'

I'm all for API parity where possible to help people easily move between tools and I think this falls under that remit

BPScott avatar Oct 31 '19 02:10 BPScott

Also note that without the --no-eslintrc flag any rules specified are merged into any existing configurations

ntwb avatar Oct 31 '19 04:10 ntwb

I'm curious what @stylelint/contributors think.

Funnily enough, I needed this feature earlier today when working through https://github.com/stylelint/stylelint-demo/issues/27#issuecomment-573317547 as I would've liked to have done:

% echo "a:b(a:(f)) {}" | npx stylelint --rule 'selector-no-vendor-prefix: true' --no-stylelintrc

I'd be happy to see the same --rule behaviour as ESLint brought to stylelint. As @ntwb said:

Also note that without the --no-eslintrc flag any rules specified are merged into any existing configurations


Shall I mark as an enhancement and help wanted?

jeddy3 avatar Jan 11 '20 17:01 jeddy3

IMO ruleFilter should accept either array or string and correspondingly we would have --rule (single rule but reusable several times) and --rules which would accept a file (JSON array) or a string (stringified array).

Mouvedia avatar Mar 16 '22 10:03 Mouvedia

For the usability side: I frequently use stylelint as a rule-to-prevent-it-in-the-future as well as a codemod to fix current issues. I'm working in a very large monorepo where I can't go in and run --fix on everything at once because it would hit too many rules across the board and touch more code than I could verify. Having the ability to select only one rule to run would be hugely helpful in this case.

betaorbust avatar Oct 04 '22 00:10 betaorbust

Thanks for chiming in with your use case, @betaorbust. It makes sense.

If anyone has time to implement these new options (--rule and --no-stylelintrc), please consider contributing.

--rule:

  • by default, merges rule to the end of the configuration object
  • can be used multiple times

--no-stylelintrc:

  • don't use the .stylelintrc file, and therefore make --rule(s) standalone

jeddy3 avatar Oct 04 '22 06:10 jeddy3

I now reconsider this issue.

--no-stylelintrc

Users can specify not only .stylelintrc* but also stylelint.config.* files. Additionally, they can use the --config flag to specify any file. So I'm concerned that the new flag name --no-stylelintrc would not be very intuitive.

Instead, --no-config seems more intuitive to me. What do you think?

FYI. ESLint has started using the new config file eslint.config.js recently.

--rule

The ESLint's --rule flag adopts the levn format. This format is more human-readable than JSON, but we need to add a new dependency on the levn library. Is it acceptable? Also, what do you think about the learning cost of users for the format?

For another point, if using --rule for a rule that long options like declaration-property-value-allowed-list, would it not difficult?

ybiquitous avatar Nov 15 '22 12:11 ybiquitous

Reconsidering this issue's original motivation, it seems that the following option would be just required:

# Run with only rules defined in an existing config file (e.g. .stylelintrc.js)
stylelint --rule declaration-property-value-allowed-list "src/**/*.css"

# Or --only may be more intuitive than --rule
stylelint --only declaration-property-value-allowed-list "src/**/*.css"
// .stylelintrc.js
module.exports = {
  rules: {
    // other rules ...

    "declaration-property-value-allowed-list": {
      "transform": ["/scale/"],
      "whitespace": "nowrap",
      "/color/": ["/^green/"],
      // other values ...
    },

    // other rules ...
  }
};

ybiquitous avatar Nov 15 '22 12:11 ybiquitous

--no-config

👍 (or --no-cfg)

--only

👎 (see https://github.com/stylelint/stylelint/issues/4331#issuecomment-1068983735)

Mouvedia avatar Nov 15 '22 12:11 Mouvedia

We should have switched this issue back to needs: discussion because there are unanswered concerns and questions.

It feels like a 3rd party stylelint-nibble package is more appropriate, as there are only a couple of hearts/thumb-ups in the four years that the issue has been open, and we're moving towards simplifying our configuration model rather than adding more means to augment it.

@Mouvedia It's fantastic that you're eager to contribute and move Stylelint forward. But, as with all these historical issues, it's best to ask if they're still relevant before picking them up as many are still open (and labelled with ready to implement) simply because we've not had time to revisit them. For example, the issue predates overrides.

jeddy3 avatar Dec 19 '23 18:12 jeddy3

@Mouvedia It's fantastic that you're eager to contribute and move Stylelint forward. But, as with all these historical issues, it's best to ask if they're still relevant before picking them up as many are still open (and labelled with ready to implement) simply because we've not had time to revisit them.

It's the second time that I have picked up an issue which status has been reverted from ready to implement to needs discussion. Minutes before I was about to turn the draft to ready to review I saw your message. I don't know what to think.

See also https://github.com/stylelint/stylelint/pull/7252#issuecomment-1849021378 which prompted me to restart my efforts.

Mouvedia avatar Dec 19 '23 18:12 Mouvedia

@Mouvedia Sorry that my comment confused you.

As @jeddy3 suggested, let's stop this now. We've recently started considering a new configuration system and CLI defaults (#7408 and #7409). The situation has changed.

ybiquitous avatar Dec 20 '23 01:12 ybiquitous

The sudden reversal as I send the PR is kind of weird and not motivating me nor reassuring me. The label system needs to be seriously reconsidered if you don't want to hinder contributions. From now on I will ask explicitly on each issue without ever trusting the labels.


I think the --rule flag is useful. It helps the effort of us being in parity with ESLint on features that were requested by the community for justified use cases.

because there are unanswered concerns and questions.

What are the unanswered concerns and questions? The only ones that I didn't cover in my PR were:

  • value-less rule picking e.g. --rule color-no-invalid-hex
  • --rules accepting a file
  • --no-cfg

Mouvedia avatar Dec 20 '23 05:12 Mouvedia

@Mouvedia Sorry for the sudden reversal.

But you still have not answered my question (https://github.com/stylelint/stylelint/pull/7252#issuecomment-1849021378) since I was surprised by the --merge-rules suddenly came. You should have confirmed that the long-standing issue was still active and decisions were unchanged before sending a PR.

ybiquitous avatar Dec 20 '23 13:12 ybiquitous

…since I was surprised by the --merge-rules suddenly came.

I can reduce the scope of the PR if you want. But IMHO this feature needs to reach a minimum threshold to be really useful. Namely:

  • being able to switch the strategy (merge or isolate)
  • being able to provide a list instead of repeating --rule dozen of times
  • handling conflicts (i.e. precedence/priority/overriding)
  • providing the same level of granularity for the node options

What I am unsure about is whether I really need levn or not. It adds complexity, lacks a declaration file and is either too loose or too strict.

You should have confirmed that the long-standing issue was still active and decisions were unchanged before sending a PR.

I was trusting the label. More than @jeddy3's reversal, it's his timing that really surprised me.

Mouvedia avatar Dec 20 '23 14:12 Mouvedia

@Mouvedia I believe there is still room to discuss new CLI flags and syntax for configuring a rule before entering implementation details.

Also, if a 3rd-party package would do the same instead, we'd not need to increase code or dependency. We have to consider if the Stylelint core should provide this feature or if it'd be worth it.

ybiquitous avatar Dec 20 '23 15:12 ybiquitous

I intended to catch this before more work was done. Unfortunately, I was too late. The timing was coincidental.

As I mentioned in the --ext issue, we need to revisit our historical issues, but no one has found the time to do that yet. We could rename the status: ready to implement label to something like status: ask to implement as a quick buffer.

We can also create a new issue to look into other ways of improving how we manage issues and contributions, e.g. automatically relabelling issues when they reach a certain age.

jeddy3 avatar Dec 20 '23 23:12 jeddy3

We could rename the status: ready to implement label to something like status: ask to implement as a quick buffer.

We could do that and slightly emend the issue template.

@jeddy3 should I close this issue and its PR?

Mouvedia avatar Jan 07 '24 17:01 Mouvedia

Yes, I think so. A 3rd-party package would help us avoid increasing our code and dependencies.

I've renamed the label.

jeddy3 avatar Jan 07 '24 19:01 jeddy3