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 CLI to avoid waiting for stdin without any input and avoid different outputs #7131

Merged
merged 3 commits into from Aug 14, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 5 additions & 0 deletions .changeset/orange-dancers-compete.md
@@ -0,0 +1,5 @@
---
"stylelint": minor
---

Changed: CLI to avoid waiting for stdin without any input
5 changes: 5 additions & 0 deletions .changeset/twelve-ladybugs-mate.md
@@ -0,0 +1,5 @@
---
"stylelint": patch
---

Fixed: CLI to avoid different outputs on empty files and empty stdin
14 changes: 14 additions & 0 deletions docs/migration-guide/to-16.md
Expand Up @@ -13,6 +13,20 @@ Node.js 14 has reached end-of-life. We've removed support for it so that we coul
- 16.13.0
- 18.0.0

## Changed CLI to avoid waiting for stdin without any input

If you run the CLI without any input files or input from stdin, the CLI immediately exits with some message (help, error, etc.). Before 16.0.0, the CLI waited for input from stdin.

```sh
# Before 16.0.0
stylelint
# waiting for stdin...

# After 16.0.0
stylelint
# show help and then exit
```

## Changed CLI to print problems to stderr instead of stdout

If you use the CLI to fix a source string by using the [`--fix`](../user-guide/cli.md#--fix) and [`--stdin`](../user-guide/cli.md#--stdin) options, the CLI will print the fixed code to stdout and any problems to stderr.
Expand Down
52 changes: 38 additions & 14 deletions lib/__tests__/cli.test.mjs
Expand Up @@ -183,24 +183,22 @@ describe('buildCLI', () => {
});

describe('CLI', () => {
beforeAll(() => {
beforeEach(() => {
Copy link
Member Author

Choose a reason for hiding this comment

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

[note] To avoid effects on each it.

jest.spyOn(process, 'exit').mockImplementation(() => {});
jest.spyOn(process.stdout, 'write').mockImplementation(() => {});
jest.spyOn(process.stderr, 'write').mockImplementation(() => {});
jest.spyOn(process, 'stdin', 'get').mockImplementation(() => Readable.from([Buffer.from('')]));
jest.spyOn(console, 'log').mockImplementation(() => {});
jest.spyOn(console, 'error').mockImplementation(() => {});
});

afterEach(() => {
jest.restoreAllMocks();
process.exitCode = undefined;
});

afterAll(() => {
jest.restoreAllMocks();
});
it('no arguments', async () => {
jest.spyOn(process, 'stdin', 'get').mockImplementationOnce(() => ({ isTTY: true }));

it('basic', async () => {
await cli([]);

expect(process.exit).toHaveBeenCalledWith(0);
Expand All @@ -223,7 +221,7 @@ describe('CLI', () => {
it('--version', async () => {
await cli(['--version']);

expect(process.exitCode).toBeUndefined();
expect(process.exit).toHaveBeenCalledWith(0);

expect(console.log).toHaveBeenCalledTimes(1);
expect(console.log).toHaveBeenCalledWith(expect.stringContaining(pkg.version));
Expand Down Expand Up @@ -303,19 +301,46 @@ describe('CLI', () => {
);
});

it('--stdin', async () => {
it('--stdin with any input from stdin', async () => {
jest.spyOn(process, 'stdin', 'get').mockImplementationOnce(() => Readable.from([]));

await cli(['--stdin', '--config', fixturesPath('config-no-empty-source.json')]);

expect(process.exitCode).toBe(2);

expect(process.stdout.write).not.toHaveBeenCalled();
expect(process.stderr.write).toHaveBeenCalledTimes(1);
expect(process.stderr.write).toHaveBeenNthCalledWith(
1,
expect(process.stderr.write).toHaveBeenCalledWith(
expect.stringContaining('Unexpected empty source'),
);
});

it('--stdin with no input from stdin', async () => {
jest.spyOn(process, 'stdin', 'get').mockImplementationOnce(() => ({ isTTY: true }));

await cli(['--stdin']);

expect(process.exitCode).toBe(1);

expect(process.stdout.write).not.toHaveBeenCalled();
expect(process.stderr.write).toHaveBeenCalledTimes(1);
expect(process.stderr.write).toHaveBeenCalledWith(
expect.stringContaining('You must pass stylelint a `files` glob or a `code` string'),
);
});

it('empty input from stdin without --stdin', async () => {
jest.spyOn(process, 'stdin', 'get').mockImplementation(() => Readable.from([]));

await cli(['--config', fixturesPath('config-no-empty-source.json')]);

expect(process.exitCode).toBe(2);

expect(process.stdout.write).not.toHaveBeenCalled();
expect(process.stderr.write).toHaveBeenCalledTimes(1);
expect(process.stderr.write).toHaveBeenCalledWith(expect.stringMatching('no-empty-source'));
});

it('exits with non zero on unfound module in config', async () => {
await cli([
'--config',
Expand Down Expand Up @@ -365,8 +390,8 @@ describe('CLI', () => {

expect(process.exitCode).toBeUndefined();

expect(process.stdout.write).toHaveBeenCalledTimes(0);
expect(process.stderr.write).toHaveBeenCalledTimes(0);
expect(process.stdout.write).not.toHaveBeenCalled();
expect(process.stderr.write).not.toHaveBeenCalled();
Copy link
Member Author

Choose a reason for hiding this comment

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

[note] To clarify the difference from toHaveBeenCalledWith(0).

});

it('--quiet-deprecation-warnings', async () => {
Expand All @@ -379,7 +404,7 @@ describe('CLI', () => {

expect(process.exitCode).toBe(2);

expect(process.stdout.write).toHaveBeenCalledTimes(0);
expect(process.stdout.write).not.toHaveBeenCalled();
expect(process.stderr.write).toHaveBeenCalledTimes(1);
expect(process.stderr.write.mock.calls[0][0]).toEqual(
expect.not.stringContaining('The "color-hex-case" rule is deprecated.'),
Expand Down Expand Up @@ -418,7 +443,6 @@ describe('CLI', () => {
expect(process.exitCode).toBe(2);

expect(process.stdout.write).not.toHaveBeenCalled();
expect(process.stdout.write).toHaveBeenCalledTimes(0);
expect(process.stderr.write).toHaveBeenCalledTimes(1);
expect(stripAnsi(process.stderr.write.mock.calls[0][0])).toContain(
'Invalid option "--globby-options". The value "wrong" is not valid JSON object.',
Expand Down
12 changes: 9 additions & 3 deletions lib/cli.mjs
Expand Up @@ -497,7 +497,7 @@ export default async function main(argv) {
return;
}

if (!options.files && !options.code && !stdin) {
if (!options.files && !isString(options.code) && !stdin) {
showHelp();

return;
Expand Down Expand Up @@ -580,12 +580,18 @@ function parseGlobbyOptions(value) {
}

/**
* @returns {Promise<string>}
* @returns {Promise<string | undefined>}
*/
async function getStdin() {
const { stdin } = process;

if (stdin.isTTY) {
Mouvedia marked this conversation as resolved.
Show resolved Hide resolved
return undefined;
}

const chunks = [];

for await (const chunk of process.stdin) {
for await (const chunk of stdin) {
chunks.push(chunk);
}

Expand Down