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

--warn-as-error prevents next stage from running if it finds any warning #2972

Closed
timotheeguerin opened this issue Mar 1, 2024 · 5 comments · Fixed by #2983
Closed

--warn-as-error prevents next stage from running if it finds any warning #2972

timotheeguerin opened this issue Mar 1, 2024 · 5 comments · Fixed by #2983
Assignees
Milestone

Comments

@timotheeguerin
Copy link
Member

The compiler will not move to the next stage if it finds an error. However when we use --warn-as-error this update the diagnostic to be an error immediately at report type not when displaying it at the end.

This is makes the --warn-as-error have a worse experience as it can't keep going if some minor elements that shouldn't prevent the next stage from running were found.

@tjprescott
Copy link
Member

If you run tsp compile . --warn-as-error, which we do in CI, and your spec has compiler warnings (think onValidate) as well as rule violations, the conpiler will stop, only emit the compiler warnings, fail, and exit without actually running the rules. This is manifesting on CI where the compile step exits due to a deprecation warning and thus doesn't run the actual linter rules.

--warn-as-error should collect all diagnostics, from the program AND rules, and only then apply --warn-as-error

@tjprescott
Copy link
Member

Also, in discussion with @timotheeguerin, --warn-as-error makes warning violations appear as errors in the output. It seems to me that it should display as the correct thing (warning or error) but simply exit with code 1 after all program and linter diagnostics have been collected if there are any warnings or errors.

@timotheeguerin
Copy link
Member Author

There is a few options:

  1. Only patch the diagnostic at the end of the compile. This has the cons that people might get an inconsitent behavior when looking at the diagnostics
  2. Make sure we don't count a warning as an error for deciding to go to the next stage with an extra flag/state
  3. Stop converting warnings to errors and instead just return non-zero exit code if --warn-as-error is specified. This has the advantage of still exposing the different status to the user and not confuse them with things that could be suppressed.

@tjprescott
Copy link
Member

I'd vote for 3.

@markcowl markcowl added this to the [2024] April milestone Mar 4, 2024
@markcowl
Copy link
Contributor

markcowl commented Mar 4, 2024

est: 2 for implementing the second option

@markcowl markcowl changed the title --warn-as-error prevents next stage from running if it fins any warning --warn-as-error prevents next stage from running if it finds any warning Mar 4, 2024
@timotheeguerin timotheeguerin self-assigned this Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants