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

RFC: make UndefVarError messages more precise and informative #51979

Merged
merged 1 commit into from
Nov 8, 2023

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Nov 1, 2023

Record the 'scope' of the variable that was undefined (the Module, or a descriptive word such as :local or :static_parameter). Add that scope to the error message, and expand the hint suggestions added by the REPL to include more specific advice on common mistakes:

  • forgetting to set an initial value
  • forgetting to import a global
  • creating a local of the same name as a global
  • not matching a static parameter in a signature subtype

Fixes #17062 (although more could probably be done to search for typos using REPL.string_distance and getting the method from stacktrace)
Fixes #18877
Fixes #25263
Fixes #35126
Fixes #39280
Fixes #41728
Fixes #48731
Fixes #49917
Fixes #50369

(this needs a fix to a Distributed test, which can be done concurrent with any discussion about exact wording here)

@vtjnash vtjnash added the error messages Better, more actionable error messages label Nov 1, 2023
@JeffBezanson
Copy link
Member

This is great! 🎉

vtjnash added a commit to JuliaLang/Distributed.jl that referenced this pull request Nov 2, 2023
vtjnash added a commit to JuliaLang/Distributed.jl that referenced this pull request Nov 3, 2023
vtjnash added a commit to JuliaLang/Distributed.jl that referenced this pull request Nov 3, 2023
Record the 'scope' of the variable that was undefined (the Module, or a
descriptive word such as :local or :static_parameter). Add that scope to
the error message, and expand the hint suggestions added by the REPL to
include more specific advice on common mistakes:

  - forgetting to set an initial value
  - forgetting to import a global
  - creating a local of the same name as a global
  - not matching a static parameter in a signature subtype

Fixes #17062 (although more could probably be done to search for typos using REPL.string_distance and getting the method from stacktrace)
Fixes #18877
Fixes #25263
Fixes #35126
Fixes #39280
Fixes #41728
Fixes #48731
Fixes #49917
Fixes #50369
@vtjnash vtjnash force-pushed the jn/undefvarmessage branch from 434809b to b985793 Compare November 8, 2023 14:20
@vtjnash vtjnash marked this pull request as ready for review November 8, 2023 14:20
@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Nov 8, 2023
@vtjnash vtjnash merged commit 449c7a2 into master Nov 8, 2023
8 checks passed
@vtjnash vtjnash deleted the jn/undefvarmessage branch November 8, 2023 17:24
@giordano giordano removed the merge me PR is reviewed. Merge when all tests are passing label Nov 8, 2023
giordano pushed a commit that referenced this pull request Oct 19, 2024
…st_throws (#56231)

Fix #54082 

Arguably this was a breaking change (as a consequence of
#51979). But regardless, it seems
like useful functionality to have a public API for testing that an
`UndefVarError` was thrown for the expected variable name (regardless of
scope). This is particularly useful if you don't know what the scope is
(for example, in my use-case i want to test that a specific
`UndefVarError` is thrown from a module with a `gensym`'d name).

Pre-v1.11 the syntax for this was 
```julia
@test_throws UndefVarError(:x) foo()
```
but that stopped working in v1.11 when `UndefVarError` got a second
field (in fact in v1.11.1 this is an error when before it would pass)

This PR restores that functionality.

We might want to backport it to v1.11.x so that v1.11 isn't the only
version that doesn't support this.
giordano pushed a commit that referenced this pull request Oct 19, 2024
…st_throws (#56231)

Fix #54082

Arguably this was a breaking change (as a consequence of
#51979). But regardless, it seems
like useful functionality to have a public API for testing that an
`UndefVarError` was thrown for the expected variable name (regardless of
scope). This is particularly useful if you don't know what the scope is
(for example, in my use-case i want to test that a specific
`UndefVarError` is thrown from a module with a `gensym`'d name).

Pre-v1.11 the syntax for this was
```julia
@test_throws UndefVarError(:x) foo()
```
but that stopped working in v1.11 when `UndefVarError` got a second
field (in fact in v1.11.1 this is an error when before it would pass)

This PR restores that functionality.

We might want to backport it to v1.11.x so that v1.11 isn't the only
version that doesn't support this.

(cherry picked from commit b0c1525)
KristofferC pushed a commit that referenced this pull request Oct 21, 2024
…st_throws (#56231)

Fix #54082 

Arguably this was a breaking change (as a consequence of
#51979). But regardless, it seems
like useful functionality to have a public API for testing that an
`UndefVarError` was thrown for the expected variable name (regardless of
scope). This is particularly useful if you don't know what the scope is
(for example, in my use-case i want to test that a specific
`UndefVarError` is thrown from a module with a `gensym`'d name).

Pre-v1.11 the syntax for this was 
```julia
@test_throws UndefVarError(:x) foo()
```
but that stopped working in v1.11 when `UndefVarError` got a second
field (in fact in v1.11.1 this is an error when before it would pass)

This PR restores that functionality.

We might want to backport it to v1.11.x so that v1.11 isn't the only
version that doesn't support this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment