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

Throw error on non-existent files unless allow-empty-input is enabled #3965

Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion flow-typed/stylelint.js
Expand Up @@ -147,5 +147,6 @@ export type stylelint$standaloneOptions = {
customSyntax?: string,
formatter?: "compact" | "json" | "string" | "unix" | "verbose" | Function,
disableDefaultIgnores?: boolean,
fix?: boolean
fix?: boolean,
allowEmptyInput: boolean
};
71 changes: 56 additions & 15 deletions lib/__tests__/ignore.test.js
Expand Up @@ -191,8 +191,11 @@ describe("extending config with ignoreFiles glob ignoring one by negation", () =
});

describe("specified `ignorePath` file ignoring one file", () => {
let results;
const noFilesErrorMessage = new Error(
"The specified `files` glob returns no results"
Copy link
Member

Choose a reason for hiding this comment

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

Shall we adopt the ESLint approach of?:

No files matching the pattern "${pattern}" were found.

This feels a little more explicit as we're stating the glob pattern in the error message, rather than just referring to it.

);
let actualCwd;
let errorMessage;

beforeAll(() => {
actualCwd = process.cwd();
Expand All @@ -212,17 +215,26 @@ describe("specified `ignorePath` file ignoring one file", () => {
}
},
ignorePath: path.join(__dirname, "fixtures/ignore.txt")
}).then(data => (results = data.results));
})
.then(() => {
throw new Error("should not have succeeded");
})
.catch(actualError => {
errorMessage = actualError;
});
});

it("no files read", () => {
expect(results).toHaveLength(0);
expect(errorMessage).toEqual(noFilesErrorMessage);
});
});

describe("specified `ignorePattern` file ignoring one file", () => {
let results;
const noFilesErrorMessage = new Error(
"The specified `files` glob returns no results"
);
let actualCwd;
let errorMessage;

beforeAll(() => {
actualCwd = process.cwd();
Expand All @@ -242,17 +254,26 @@ describe("specified `ignorePattern` file ignoring one file", () => {
}
},
ignorePattern: "fixtures/empty-block.css"
}).then(data => (results = data.results));
})
.then(() => {
throw new Error("should not have succeeded");
})
.catch(actualError => {
errorMessage = actualError;
});
});

it("no files read", () => {
expect(results).toHaveLength(0);
expect(errorMessage).toEqual(noFilesErrorMessage);
});
});

describe("specified `ignorePattern` file ignoring two files", () => {
let results;
const noFilesErrorMessage = new Error(
"The specified `files` glob returns no results"
);
let actualCwd;
let errorMessage;

beforeAll(() => {
actualCwd = process.cwd();
Expand All @@ -278,11 +299,17 @@ describe("specified `ignorePattern` file ignoring two files", () => {
"fixtures/empty-block.css",
"fixtures/no-syntax-error.css"
]
}).then(data => (results = data.results));
})
.then(() => {
throw new Error("should not have succeeded");
})
.catch(actualError => {
errorMessage = actualError;
});
});

it("no files read", () => {
expect(results).toHaveLength(0);
expect(errorMessage).toEqual(noFilesErrorMessage);
});
});

Expand Down Expand Up @@ -416,6 +443,9 @@ describe("using codeFilename and ignoreFiles with configBasedir", () => {
});

describe("file in node_modules", () => {
const noFilesErrorMessage = new Error(
"The specified `files` glob returns no results"
);
const lint = () =>
standalone({
files: [`${fixturesPath}/node_modules/test.css`],
Expand All @@ -427,13 +457,20 @@ describe("file in node_modules", () => {
});

it("is ignored", () => {
return lint().then(output => {
expect(output.results).toHaveLength(0);
});
return lint()
.then(() => {
throw new Error("should not have succeeded");
})
.catch(actualError => {
expect(actualError).toEqual(noFilesErrorMessage);
});
});
});

describe("file in bower_components", () => {
const noFilesErrorMessage = new Error(
"The specified `files` glob returns no results"
);
const lint = () =>
standalone({
files: [`${fixturesPath}/bower_components/test.css`],
Expand All @@ -445,9 +482,13 @@ describe("file in bower_components", () => {
});

it("is ignored", () => {
return lint().then(output => {
expect(output.results).toHaveLength(0);
});
return lint()
.then(() => {
throw new Error("should not have succeeded");
})
.catch(actualError => {
expect(actualError).toEqual(noFilesErrorMessage);
});
});
});

Expand Down
20 changes: 19 additions & 1 deletion lib/__tests__/standalone.test.js
Expand Up @@ -114,10 +114,28 @@ it("standalone without input css and file(s) should throw error", () => {
);
});

it("standalone with non-existent-file quietly exits", () => {
it("standalone with non-existent-file throws an error", () => {
const expectedError = new Error(
"The specified `files` glob returns no results"
);

return standalone({
files: `${fixturesPath}/non-existent-file.css`,
config: configBlockNoEmpty
})
.then(() => {
throw new Error("should not have succeeded");
Copy link
Member

Choose a reason for hiding this comment

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

I'm just curious why to throw an error here? Isn't stylelint should throw an error and catch will catch it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi, I removed this, it was left over from testing

})
.catch(actualError => {
expect(actualError).toEqual(expectedError);
});
});

it("standalone with non-existent-file and allowEmptyInput enabled quietly exits", () => {
return standalone({
files: `${fixturesPath}/non-existent-file.css`,
config: configBlockNoEmpty,
allowEmptyInput: true
}).then(linted => {
expect(typeof linted.output).toBe("string");
expect(linted.results).toHaveLength(0);
Expand Down
14 changes: 12 additions & 2 deletions lib/cli.js
Expand Up @@ -125,7 +125,8 @@ const EXIT_CODE_ERROR = 2;
stdinFilename: any,
syntax: any,
v: string,
version: string
version: string,
allowEmptyInput: boolean
},
input: any,
help: any,
Expand Down Expand Up @@ -153,7 +154,8 @@ const EXIT_CODE_ERROR = 2;
maxWarnings?: any,
syntax?: any,
disableDefaultIgnores?: any,
ignorePattern?: any
ignorePattern?: any,
allowEmptyInput?: boolean
}*/

const meowOptions /*: meowOptionsType*/ = {
Expand Down Expand Up @@ -278,6 +280,10 @@ const meowOptions /*: meowOptionsType*/ = {
--version, -v

Show the currently installed version of stylelint.

--alow-empty-input
Bilie marked this conversation as resolved.
Show resolved Hide resolved

When glob pattern matches no files, the process will exit without throwing an error.
`,
flags: {
cache: {
Expand Down Expand Up @@ -478,6 +484,10 @@ module.exports = (argv /*: string[]*/) /*: Promise<void>|void*/ => {
return;
}

if (cli.flags.allowEmptyInput) {
optionsBase.allowEmptyInput = cli.flags.allowEmptyInput;
}

return Promise.resolve()
.then(() => {
// Add input/code into options
Expand Down
5 changes: 5 additions & 0 deletions lib/standalone.js
Expand Up @@ -42,6 +42,7 @@ module.exports = function(
const reportNeedlessDisables = options.reportNeedlessDisables;
const maxWarnings = options.maxWarnings;
const syntax = options.syntax;
const allowEmptyInput = options.allowEmptyInput || false;
const useCache = options.cache || false;
let fileCache;
const startTime = Date.now();
Expand Down Expand Up @@ -198,6 +199,10 @@ module.exports = function(
);

if (!filePaths.length) {
if (!allowEmptyInput) {
throw new Error("The specified `files` glob returns no results");
}

return Promise.all([]);
}

Expand Down