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

integrate unused-variable (and other unused-*) warnings with ordinary type-error messages #5903

Closed
vicuna opened this issue Jan 23, 2013 · 8 comments

Comments

@vicuna
Copy link

vicuna commented Jan 23, 2013

Original bug ID: 5903
Reporter: sweeks
Status: acknowledged (set by @damiendoligez on 2013-06-19T18:42:28Z)
Resolution: open
Priority: normal
Severity: feature
Target version: undecided
Category: typing
Parent of: #6580
Monitored by: @hcarty @yakobowski

Bug description

I think it would be preferable if unused-variable warnings appeared in
the same order and interspersed with ordinary type-error messages,
rather than all appearing after the ordinary type errors. They
sometimes provide information that is helpful in resolving type
errors. And, this would allow one to focus on one piece of code and
resolve errors in it at the same time and move on, rather than having
to jump back to a piece of code that you thought had passed already
the type checker.

@vicuna
Copy link
Author

vicuna commented Jan 24, 2013

Comment author: @alainfrisch

Warnings on unused stuff need to wait until type-checking is done, so the only solution I can see is to keep all warnings in a buffer, and sort them by location before displaying them (and be careful to do that even in case of type error). If someone wants to propose a patch...

@vicuna
Copy link
Author

vicuna commented Jan 24, 2013

Comment author: @yakobowski

I'm not convinced that this would be useful. Some warnings used to be intermingled with errors, and it was quite painful to skip through them before reaching the real error. In particular, when you're developing new code, warnings for unused things are bound to happen (partially written functions, functions not called yet, etc), and I clearly do not want to see them before a typing error.

@vicuna
Copy link
Author

vicuna commented Jan 24, 2013

Comment author: @alainfrisch

I clearly do not want to see them before a typing error.

Oh, don't worry I don't think there is a plan to do that. Currently the warnings require the whole source to be type-checked (because they are no longer done syntactically on the Parsetree), but the type-checker stops at the first error. In some cases, it might be possible to detect before the end of the type-checking that some stuffs are definitely unused (and so be able to report it even in case of a type-error later), but this is quite difficult to do.

My understanding of the feature wish was that those unused-stuff warnings should appear "between" other "warnings turned into errors", not real type errors.

@vicuna
Copy link
Author

vicuna commented Jan 19, 2015

Comment author: sweeks

I want the error reporting mechanism to not distinguish between unused-variable errors and type errors, and would like them to be reported with the same priority -- I would like all errors to be reported in the same way, sorted by line number. I don't want certain kinds of errors to be skipped. I don't care what the default behavior is, as long as there is a way to configure this behavior.

@vicuna
Copy link
Author

vicuna commented Jan 19, 2015

Comment author: @alainfrisch

I don't think it is possible to achieve what you want as long as the type-checker stops on the first error. The unused-variables "errors" can only be reported when the scope of the variable is fully type-checked, so when this scope contains (another) type error, you cannot get the unused-variable error. The only way I can see to achieve what you want would be to rely on a syntactic approximation of the unused-variable criterion i.e. the old algorithm to implement those warnings. The new approach is more precise and adding back support for the old one (in addition to the new one) seems to add too much complexity for the gain.

@vicuna
Copy link
Author

vicuna commented Jan 19, 2015

Comment author: sweeks

I can imagine there would be some trickiness to implement this for module-level variables, but for local variables, I would have thought it would be easy and that there would be a natural place where the type-checking code is finished processing the scope of a local variable, and could report at that point if the variable is unused.

Clearly not a big issue.

@vicuna
Copy link
Author

vicuna commented Mar 7, 2017

Comment author: @mshinwell

I'd like to add support for improving this area. Unused variable / parameter errors are often very useful for finding errors in code that otherwise cause obscure type error messages. I hit one of these just yesterday. I wanted to remove the first parameter of a toplevel function, which I did everywhere, except actually in the "let foo = ..." line by accident. This caused an obscure error message in the next toplevel function, when in fact, it should have just said "unused parameter ..." when it had finished type checking "foo". This probably wasted ten or fifteen minutes.

@github-actions
Copy link

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant