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

Fix false positives for variables in font-family-no-missing-generic-family-keyword #4765

Closed
jeddy3 opened this issue May 12, 2020 · 4 comments · Fixed by #4806 or #5240
Closed

Fix false positives for variables in font-family-no-missing-generic-family-keyword #4765

jeddy3 opened this issue May 12, 2020 · 4 comments · Fixed by #4806 or #5240
Labels
good first issue is good for newcomers status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule

Comments

@jeddy3
Copy link
Member

jeddy3 commented May 12, 2020

Clearly describe the bug

There are false positives in font-family-no-missing-generic-family-keyword if I use a variable and a font family.

Demo.

See #3848 (comment).

Which rule, if any, is the bug related to?

font-family-no-missing-generic-family-keyword

What code is needed to reproduce the bug?

a {
  font-family: 'font', var(--font);
}

What stylelint configuration is needed to reproduce the bug?

{
  "rules": {
     "font-family-no-missing-generic-family-keyword": [true, 
       { "ignoreFontFamilies": ["var(--font)"] }
     ]
  }
}

Which version of stylelint are you using?

13.3.0

How are you running stylelint: CLI, PostCSS plugin, Node.js API?

Demo

Does the bug relate to non-standard syntax (e.g. SCSS, Less etc.)?

No

What did you expect to happen?

No warnings to be flagged because the rule should ignore values that contain variables.

What actually happened (e.g. what warnings or errors did you get)?

The following warnings were flagged:

3:16 error Unexpected missing generic font family (font-family-no-missing-generic-family-keyword)
@jeddy3 jeddy3 added status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule good first issue is good for newcomers labels May 12, 2020
@jeddy3
Copy link
Member Author

jeddy3 commented May 12, 2020

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

There are steps on how to fix a bug in a rule in the Developer guide.

srawlins added a commit to srawlins/stylelint that referenced this issue May 27, 2020
This introduces a second parse loop through a font-family.

To avoid this, we could add an parameter to findFontFamily which
dictates whether to include variables in the result, but the function
would need to blindly include variables, without sanity checking them
(like whether a variable represents a number or something).

Fixes stylelint#4765
@lensbart
Copy link

lensbart commented Apr 11, 2021

The same issue still persists with Sass variables (e.g. font-family: 'Comic Sans', $fontFamilyUI;). I’ve tried several permutations in the ignoreFontFamilies array: using a string, regex with escaped $ character, string-of-regex with twice-escaped \\, etc; but nothing seems to work.

// stylelint.config.js
module.exports = {
  rules: {
    'font-family-no-missing-generic-family-keyword': [
      true,
      { ignoreFontFamilies: [/\$fontFamilyUI/] }
    ]
  }
}

@ybiquitous
Copy link
Member

@lensbart Thanks for the report. We can reproduce it on the demo.

@ybiquitous ybiquitous reopened this Apr 12, 2021
ybiquitous added a commit that referenced this issue Apr 12, 2021
…by default

This change aims to ignore any variables (e.g. `var()`, `$var`, `@var` etc.) by default
for the `font-family-no-missing-generic-family-keyword` rule.

Fix #4765 (retry of #4806)
ybiquitous added a commit that referenced this issue Apr 12, 2021
This change aims to ignore any variables (e.g. `var()`, `$var`, `@var` etc.)
for the `font-family-no-missing-generic-family-keyword` rule.

Fix #4765 (retry of #4806)
@ybiquitous
Copy link
Member

I've opened a new PR #5240!

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 type: bug a problem with a feature or rule
3 participants