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 some path / glob problems #4867
Changes from all commits
d4a6d41
36908d1
df33e3d
a1bb65b
e22cae5
1ef46e0
62d7360
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
a {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
a {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
a {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
a {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
a {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
a {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
a {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
a {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
a {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,193 @@ | ||
'use strict'; | ||
|
||
/* eslint-disable node/no-extraneous-require */ | ||
|
||
const describe = require('@jest/globals').describe; | ||
const expect = require('@jest/globals').expect; | ||
const it = require('@jest/globals').it; | ||
|
||
/* eslint-enable */ | ||
|
||
const path = require('path'); | ||
const replaceBackslashes = require('../testUtils/replaceBackslashes'); | ||
const standalone = require('../standalone'); | ||
|
||
const fixturesPath = replaceBackslashes(path.join(__dirname, 'fixtures', 'globs')); | ||
|
||
// Tests for https://github.com/stylelint/stylelint/issues/4521 | ||
|
||
describe('standalone globbing', () => { | ||
describe('paths with special characters', () => { | ||
// ref https://github.com/micromatch/micromatch#matching-features | ||
const fixtureDirs = [ | ||
`[digit]/not-digits`, | ||
`with spaces`, | ||
`extglob!(s)`, | ||
`got!negate/negate`, | ||
// `extglob+(s)`, // Note: +'s cause errors. Ignoring until it becomes a problem | ||
]; | ||
|
||
// https://github.com/stylelint/stylelint/issues/4193 | ||
it.each(fixtureDirs)(`static path contains "%s"`, async (fixtureDir) => { | ||
const cssPath = `${fixturesPath}/${fixtureDir}/styles.css`; | ||
|
||
const { results } = await standalone({ | ||
files: cssPath, | ||
config: { rules: { 'block-no-empty': true } }, | ||
}); | ||
|
||
expect(results).toHaveLength(1); | ||
expect(results[0].errored).toEqual(true); | ||
expect(results[0].warnings[0]).toEqual( | ||
expect.objectContaining({ | ||
rule: 'block-no-empty', | ||
severity: 'error', | ||
}), | ||
); | ||
}); | ||
}); | ||
|
||
// https://github.com/stylelint/stylelint/issues/4211 | ||
it('glob has no + character, matched path does', async () => { | ||
const files = `${fixturesPath}/**/glob-plus.css`; // file is in dir 'glob+chars' | ||
|
||
const { results } = await standalone({ | ||
files, | ||
config: { rules: { 'block-no-empty': true } }, | ||
}); | ||
|
||
expect(results).toHaveLength(1); | ||
expect(results[0].errored).toEqual(true); | ||
expect(results[0].warnings[0]).toEqual( | ||
expect.objectContaining({ | ||
rule: 'block-no-empty', | ||
severity: 'error', | ||
}), | ||
); | ||
}); | ||
|
||
// https://github.com/stylelint/stylelint/issues/4211 | ||
it('glob contains + character, matched path does not', async () => { | ||
const files = `${fixturesPath}/+(g)lob-contains-plus/*.css`; | ||
|
||
const { results } = await standalone({ | ||
files, | ||
config: { rules: { 'block-no-empty': true } }, | ||
}); | ||
|
||
expect(results).toHaveLength(1); | ||
expect(results[0].errored).toEqual(true); | ||
expect(results[0].warnings[0]).toEqual( | ||
expect.objectContaining({ | ||
rule: 'block-no-empty', | ||
severity: 'error', | ||
}), | ||
); | ||
}); | ||
|
||
// https://github.com/stylelint/stylelint/issues/3272 | ||
// should ignore 'negated-globs/ignore/styles.css' | ||
it('negated glob patterns', async () => { | ||
const files = [ | ||
`${fixturesPath}/negated-globs/**/*.css`, | ||
`!${fixturesPath}/negated-globs/ignore/**/*.css`, | ||
]; | ||
|
||
const { results } = await standalone({ | ||
files, | ||
config: { rules: { 'block-no-empty': true } }, | ||
}); | ||
|
||
// ensure that the only result is from the unignored file | ||
expect(results[0].source).toEqual(expect.stringContaining('lint-this-file.css')); | ||
|
||
expect(results).toHaveLength(1); | ||
expect(results[0].errored).toEqual(true); | ||
expect(results[0].warnings[0]).toEqual( | ||
expect.objectContaining({ | ||
rule: 'block-no-empty', | ||
severity: 'error', | ||
}), | ||
); | ||
}); | ||
|
||
describe('mixed globs and paths with special chars', () => { | ||
it('manual escaping', async () => { | ||
const cssGlob = `${fixturesPath}/got\\[braces\\] and \\(spaces\\)/*.+(s|c)ss`; | ||
|
||
const { results } = await standalone({ | ||
files: cssGlob, | ||
config: { | ||
rules: { | ||
'block-no-empty': true, | ||
}, | ||
}, | ||
}); | ||
|
||
expect(results).toHaveLength(1); | ||
expect(results[0].errored).toEqual(true); | ||
expect(results[0].warnings[0]).toEqual( | ||
expect.objectContaining({ | ||
rule: 'block-no-empty', | ||
severity: 'error', | ||
}), | ||
); | ||
}); | ||
|
||
it('setting "cwd" in globbyOptions', async () => { | ||
const cssGlob = `*.+(s|c)ss`; | ||
|
||
const { results } = await standalone({ | ||
files: cssGlob, | ||
config: { | ||
rules: { | ||
'block-no-empty': true, | ||
}, | ||
}, | ||
globbyOptions: { | ||
cwd: `${fixturesPath}/got[braces] and (spaces)/`, | ||
}, | ||
}); | ||
|
||
expect(results).toHaveLength(1); | ||
expect(results[0].errored).toEqual(true); | ||
expect(results[0].warnings[0]).toEqual( | ||
expect.objectContaining({ | ||
rule: 'block-no-empty', | ||
severity: 'error', | ||
}), | ||
); | ||
}); | ||
|
||
/* eslint-disable jest/no-commented-out-tests -- Failing case for reference. Documents behaviour that doesn't work. */ | ||
|
||
// Note: This fails because there's no way to tell which parts of the glob are literal characters, and which are special globbing characters. | ||
// | ||
// 'got[braces] and (spaces)' is a literal directory path. `*.+(s|c)ss` is a glob. | ||
|
||
// https://github.com/stylelint/stylelint/issues/4855 | ||
// it('glob and matched path contain different special chars, complex example', async () => { | ||
// const cssGlob = `${fixturesPath}/got[braces] and (spaces)/*.+(s|c)ss`; | ||
|
||
// const { results } = await standalone({ | ||
// files: cssGlob, | ||
// config: { | ||
// rules: { | ||
// 'block-no-empty': true, | ||
// }, | ||
// }, | ||
// }); | ||
|
||
// expect(results).toHaveLength(1); | ||
// expect(results[0].errored).toEqual(true); | ||
// expect(results[0].warnings[0]).toEqual( | ||
// expect.objectContaining({ | ||
// rule: 'block-no-empty', | ||
// severity: 'error', | ||
// }), | ||
// ); | ||
// }); | ||
|
||
/* eslint-enable */ | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -118,6 +118,7 @@ | |
"cosmiconfig": "^7.0.0", | ||
"debug": "^4.1.1", | ||
"execall": "^2.0.0", | ||
"fast-glob": "^3.2.4", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is already a dependency of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just note - prettier and many other tools remove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh interesting! That could be a good follow-up PR. It would need to be a breaking change, I think. As |
||
"fastest-levenshtein": "^1.0.9", | ||
"file-entry-cache": "^5.0.1", | ||
"get-stdin": "^8.0.0", | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious, what's the error here? Should we document that there's a problem with the
+
character?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I should have made a note of it. The
+
seemed to be escaped differently for this case, causing the expected match to fail.I can't remember the specific details, but I think I tracked it down to somewhere in one of globby's dependencies. I thought I'd read an issue that described it, but now I can't find it 🤷
It seemed like enough of an edge-case that we can skip it for now, and investigate later if it becomes a problem.