prettier / prettier-eslint-cli

CLI for prettier-eslint
https://npm.im/prettier-eslint-cli
MIT License
539 stars 85 forks source link

Allow prettier-eslint-cli to write eslint errors/warnings to stderr? #453

Open gwhitney opened 8 months ago

gwhitney commented 8 months ago

Prior to prettier-eslint v. 16.2.0, it discarded all errors/warnings from eslint. Now there is an alternate entry to prettier-eslint that captures that information. I run prettier-eslint-cli automatically on save from my editor, so it would be convenient for any such warnings/errors to be written to stderr. That way, any errors would pop up in the editor when I save, which would prevent me from having a surprise that there are eslint violations when I go to try to make a commit.

Could an option be added to prettier-eslint-cli that will cause eslint errors/warnings to be written to stderr? Perhaps --report-errors ?

JounQin commented 8 months ago

Are you talking about https://github.com/prettier/prettier-eslint/pull/827?

gwhitney commented 8 months ago

Yes, that's correct. My entire motivation for making that pull request to prettier-eslint was to be able to file this proposal/issue here. I will be happy to supply a PR for prettier-eslint-cli, if the idea raised in this issue is agreeable to you.

JounQin commented 8 months ago

Cool! That's definitely a great feature to me.

About the option name I think maybe --report-issues is better? As you said, not only error are reported. And also we can support --report-issues error for filtering.

Another question, does ESLint print warning into stdout or stderror? I suppose it should be stdout, but stderror when max-warnings executed.

gwhitney commented 8 months ago

I like --report-issues. But I am unclear: is it possible to support both --report-issues and --report-issues error? How can the command-line parser then tell whether error is the argument to --report-issues or a glob argument? Or should there be a mandatory argument so that you must say either --report-issues all or --report-issues error (or --report-issues none which is the current behavior and so presumably the new default)?

As far as where the errors/warnings go, since the default behavior of prettier-eslint is to write the formatted version to stdout, it seems to me this information has to (all) go to stderr. It's also convenient because my editor is expecting nothing on stderr and if there is anything, it shows it in a popup. But if you have another convention in mind for what should go to stdout/stderr under what conditions, just let me know.

When these design points are settled I will be happy to supply a PR as soon as I am able.

JounQin commented 8 months ago

I believe --report-issues + --report-issues error can be supported at the same time. Let's give it a try.

For stdout vs stderror, like I mentioned, I hope we keep same as ESLint itself.

gwhitney commented 8 months ago

OK, I will try to figure out how to accept both --report-issues and --report-issues error. I will assume that if the word following --report-issues happens to be error, then it is interpreted as an argument to --report-issues, so that even if there is a file in the current directory named error, then prettier-eslint-cli --report-issues error will be a no-op, rather than processing the file named error with all issues being reported.

On the other hand, on stdout vs. stderror, I remain confused about "being the same as ESLint itself." The default behavior of prettier-eslint-cli is analogous to eslint --fix-dry-run, except that it writes the fixed code to stdout. As far as I can tell, there is no set of command-line arguments to eslint that makes it have this behavior, so I can't tell what it would mean to keep things "the same as ESLint itself." And as I said, since prettier-eslint-cli is writing the fixed code to stdout, it seems problematic to also write warnings/errors to stdout -- how could one distinguish in the results between fixed code and warnings/errors in that case? So I continue to suggest that in the case prettier-eslint-cli --report-issues foo.js (with no --write option) the warnings/errors should go to stderr.

On the other hand, prettier-eslint-cli --write is pretty analogous to eslint --fix, and I agree that eslint --fix writes the messages to stdout. So if you would like prettier-eslint-cli --write --report-issues foo.js to write warnings/errors to stdout, I could certainly do that. But if in the end you agree that in prettier-eslint-cli --report-issues foo.js (with no --write) the warnings/errors need to go to stderr, then does it seem possibly more consistent not to change what stream the warnings/errors go to just because of the presence of the --write option?

Let me know on this point and I will get to work on a PR implementing the feature.

JounQin commented 8 months ago

so that even if there is a file in the current directory named error, then prettier-eslint-cli --report-issues error will be a no-op, rather than processing the file named error with all issues being reported.

Hah, I didn't considered that a file named error, in this case I think we should report a warning for the user, and accept encoded "error" (just an example) as the filename to be checked.

On the other hand, on stdout vs. stderror, I remain confused about "being the same as ESLint itself."

I mean, what does eslint do when it prints warnings and errors on fixing (eslint --fix), we should just follow its behavior here.

gwhitney commented 8 months ago

Hah, I didn't considered that a file named error, in this case I think we should report a warning for the user, and accept encoded "error" (just an example) as the filename to be checked.

OK, if the word after --report-issues is error and there is no other filename/glob to check, and there is a file named error, then we will have prettier-eslint-cli process the file error but also issue a warning about the unusual situation.

I mean, what does eslint do when it prints warnings and errors on fixing (eslint --fix), we should just follow its behavior here.

OK, that's fine, that would mean that prettier-eslint-cli --write --report-issues foo.js would output warnings/errors to stdout. But that does not necessarily determine where prettier-eslint-cli --report-issues foo.js (without --write) should send warnings/errors. This case of prettier-eslint-cli is already writing the fixed code to stdout. Do you really want to mix fixed code, and warnings/errors, into the same output stream? This case of prettier-eslint-cli is basically not using stderr, and to my mind, stderr exists for the purpose of having a place to send errors/warnings when there is also "regular" output. So please confirm whether in the case without --write you wish to separate the fixed code and the errors/warnings into stdout and stderr, respectively, or actually prefer to mix them together into stdout. Thanks very much.

JounQin commented 8 months ago

OK, if the word after --report-issues is error and there is no other filename/glob to check, and there is a file named error, then we will have prettier-eslint-cli process the file error but also issue a warning about the unusual situation.

Yes, and --report-issues error error will not emit warning.

So please confirm whether in the case without --write you wish to separate the fixed code and the errors/warnings into stdout and stderr, respectively, or actually prefer to mix them together into stdout.

I want to be compliant with ESLint's output, I don't have strong opinion on whether to use stdout or stderr. I can't imagine any downsides with this policy?

gwhitney commented 8 months ago

I can't imagine any downsides with this policy?

Which policy:

A) When --write is not specified but --report-issues is, the fixed code goes to stdout and the warnings/errors go to stderr,

OR

B) When --write is not specified but --report-issues is, the fixed code and warnings/errors are mixed together on stdout?

(I have understood that when both --write and --report-issues are specified, you would like the warnings/errors to go to stdout analogous to eslint --fix, and I will implement that. But eslint has no behavior analogous to prettier-eslint-cli --report-issues without the --write option, so I need to know which of (A) or (B) above you would like me to implement. Thank you.)

JounQin commented 8 months ago

But eslint has no behavior analogous to prettier-eslint-cli --report-issues without the --write option,

I don't quite follow, isn't eslint command without --fix itself the similar thing?

What means I want when --write is not specified but --report-issues is, I want to follow the behavior of eslint command without --fix.

gwhitney commented 8 months ago

But eslint has no behavior analogous to prettier-eslint-cli --report-issues without the --write option,

I don't quite follow, isn't eslint command without --fix itself the similar thing?

No, eslint without --fix simply tells you about all questionable formatting in your code, it does not (even attempt to suggest how to) correct your code. On the other hand, prettier-eslint-cli --report-issues without the --write option would still write out the corrected code to stdout, just the idea is that it will now also tell you about the issues that it does not now how to fix (without --report-issues those issues are silenetly ignored). So the question is where should that additional information go?

What means I want when --write is not specified but --report-issues is, I want to follow the behavior of eslint command without --fix.

But as mentioned above, eslint without --fix does not provide corrections, whereas prettier-eslint-cli does always produce corrections. The closest eslint variant to what prettier-eslint-cli does is eslint --fix-dry-run -- but that does not write out the corrected code. Invoked exactly like that, all it does is write to stdout the unfixable errors, amusingly enough. There is another variant eslint --fix-dry-run --format json which writes a big JSON structure to stdout that encodes all of the errors (fixable and unfixable) as well as the proposed fixed code (in one of the top-level properties of the structure). But I don't think suddenly adding a JSON output format to prettier-eslint-cli makes sense. It is focused on correcting code; I would just like to know in addition if it noticed any unfixable problems with the code. This is not really a mode of operation of eslint, and would if implemented be a noticeable added value of prettier-eslint-cli if we can agree on an implementation. Hence, to reiterate my working proposal:

1) In the prettier-eslint-cli --write --report-errors case, which is quite analogous to eslint --fix, you would like to match eslint behavor and send the errors/warnings to stdout.

2) In the prettier-eslint-cli --report-errors case, to match the current behavior of prettier-eslint-cli, the corrected code should be written to stdout. There isn't a closely analogous mode of eslint, and so to make it easy for people using the tool to separate the suggested fixed code from the (unfixable) warnings/errors, I propose the latter be sent to stderr in this case. Such behavior doesn't really conflict with eslint, and will make it quite easy to use the tool to do whatever you want with both the fixed code and with the warning/error information -- just send stdout and stderr (respectively) wherever you like.

Please let me know if this scheme meets with your approval for a PR, or suggest concrete different behavior that you would like to see implemented. Thanks!

JounQin commented 8 months ago

@gwhitney Thanks for your patience and great writing up, I really appreciate. And I think you've convinced me. Let's print warnings/errors into stderr!

gwhitney commented 8 months ago

Great. With the holidays not sure exactly when I will be able to finish the PR, but I will try to get it done by mid-next month if not before.

JounQin commented 8 months ago

No worry, there is no deadline for this optional feature.