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

Add selector-attribute-name-disallowed-list #4915

Closed
gbhasha opened this issue Sep 1, 2020 · 5 comments
Closed

Add selector-attribute-name-disallowed-list #4915

gbhasha opened this issue Sep 1, 2020 · 5 comments
Labels
good first issue is good for newcomers status: ready to implement is ready to be worked on by someone status: wip is being worked on by someone type: new rule an entirely new rule

Comments

@gbhasha
Copy link

gbhasha commented Sep 1, 2020

What is the problem you're trying to solve?

The developers on my team use class and id attributes in attribute selectors instead of using id selector or classname selector. This adds complexity and inconsistent coding style.

What solution would you like to see?

To fix the above mentioned problem, having this linting rule would definitely help.

example use case:

selector-attribute-disallowed-list: ['class', 'id']

should throw error for below selectors....
div[class*='some-thing']
div[id~='some-thing']

@jeddy3 jeddy3 added good first issue is good for newcomers status: ready to implement is ready to be worked on by someone type: new rule an entirely new rule labels Sep 1, 2020
@jeddy3 jeddy3 changed the title Add selector-attribute-allowed-list and selector-attribute-disallowed-list rules Add selector-attribute-name-disallowed-list Sep 1, 2020
@jeddy3
Copy link
Member

jeddy3 commented Sep 1, 2020

@gbhasha Thanks for the request and for using the template.

The developers on my team use class and id attributes in attribute selectors instead of using id selector or classname selector.

That's fascinating. I can imagine why they'd want to do that; if you're already using attribute selectors for things like a[href$=".pdf"]::after { content: "(PDF)"; }, why not consistently use them for everything else including class and id attributes?

Incidentally, it's possible to enforce that approach using:

{ 
  "selector-max-id": 0,
  "selector-max-class": 0
}

Anyhow, you want to enforce the opposite and, as you said, that's not currently possible with stylelint.

having this linting rule would definitely help... selector-attribute-disallowed-list: ['class', 'id']

Yes, I agree. I suggest we go with selector-attribute-name-disallowed-list so it's clear which part of the selector we are checking. I also suggest we check it as a CSS qualified name, i.e. include any namespaces (spec ref).

I assume you'll use disallowed-list approach, rather than allowed-list? If so, let's just add the former for now.

  • Name: selector-attribute-name-disallowed-list
  • Primary option: array|string
  • Secondary options: None
  • Autofixable: No
  • Message: Unexpected name "${name}"
  • Description: "Specify a list of disallowed attribute names."
  • Section: "Limit language features" -> "Selector"

I've labelled the issue as ready to implement. Please consider contributing if you have time.

There are steps on how to add a new rule in the Developer guide.

You'll want to use the selector-attribute-operator-disallowed-list rule as a blueprint. The CSS qualified name is available in the AST as attribute.qualifiedAttribute.

@rletsin
Copy link
Contributor

rletsin commented Oct 13, 2020

Hi @jeddy3! I'd like to implement this rule as my first contribution.

@hudochenkov
Copy link
Member

@rletsin That's great! I'll mark issue as wip (work in progress).

@hudochenkov hudochenkov added the status: wip is being worked on by someone label Oct 13, 2020
@julienw
Copy link

julienw commented Nov 30, 2020

Should this issue be closed now that #4992 is merged?

BTW there is a legitimate use of an attribute selector for IDs: so that it doesn't use the higher specifity for IDs. (but better use classes everywhere :-) ).

@jeddy3
Copy link
Member

jeddy3 commented Dec 3, 2020

Should this issue be closed now that #4992 is merged?

Yes.

BTW there is a legitimate use of an attribute selector for IDs: so that it doesn't use the higher specificity for IDs.

Oh wow, I hadn't considered specificity. Neat trick.

@jeddy3 jeddy3 closed this as completed Dec 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue is good for newcomers status: ready to implement is ready to be worked on by someone status: wip is being worked on by someone type: new rule an entirely new rule
Development

No branches or pull requests

5 participants