Skip to content

Commit d731186

Browse files
committed
diagnostic: Add lowering/ambiguous-soft-scope detection
Add a new diagnostic that warns when a variable assignment inside a `for`/`while`/`try` block at the top level shadows an existing global variable, matching the warning Julia itself emits at runtime as explained in the Julia's [soft scope manual](https://docs.julialang.org/en/v1/manual/variables-and-scoping/#on-soft-scope): ``` global i = 0 while i < 5 i += 1 # Assignment to `i` in soft scope is ambiguous (JETLS lowering/ambiguous-soft-scope) # Variable `i` may be used before it is defined (JETLS lowering/undef-local-var) println(i) end ``` - Add `analyze_ambiguous_soft_scope!` using JuliaLowering's `is_ambiguous_local` binding flag - Add code actions: "Insert `global`" (preferred) and "Insert `local`" declarations with proper indentation - Enable soft scope semantics for notebook cells via `is_notebook_cell_uri`, suppressing this diagnostic
1 parent 509227b commit d731186

File tree

10 files changed

+357
-21
lines changed

10 files changed

+357
-21
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
5050
Unreachable code is displayed as faded/grayed out with the `Unnecessary` tag.
5151
A "Delete unreachable code" quick fix code action is also available.
5252
53+
- Added [`lowering/ambiguous-soft-scope`](https://aviatesk.github.io/JETLS.jl/release/diagnostic/#diagnostic/reference/lowering/ambiguous-soft-scope) diagnostic that warns when a variable assignment inside a `for`/`while`/`try` block at the top level shadows an existing global variable.
54+
This matches the warning Julia itself emits at runtime for this pattern.
55+
Two code actions are offered: "Insert `global` declaration" (preferred) and "Insert `local` declaration".
56+
This diagnostic is suppressed for notebook cells, where soft scope semantics are enabled.
57+
5358
### Changed
5459
5560
- [`lowering/undef-local-var`](https://aviatesk.github.io/JETLS.jl/release/diagnostic/#diagnostic/reference/lowering/undef-local-var) now recognizes correlated conditions to reduce false positives.

LSP/src/basic-json-structures.jl

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -504,22 +504,11 @@ Structure to capture a description for an error code.
504504
end
505505

506506
# JETLS specific data structures for `data` field of `Diagnostic`
507-
struct UnusedArgumentData
508-
is_kwarg::Bool
509-
end
510-
export UnusedArgumentData
511-
512-
struct UnusedVariableData
513-
is_tuple_unpacking::Bool
514-
assignment_range::Union{Nothing,Range}
515-
lhs_eq_range::Union{Nothing,Range}
516-
end
517-
export UnusedVariableData
518-
519-
struct UnusedImportData
520-
delete_range::Range
507+
struct AmbiguousSoftScopeData
508+
name::String
509+
indent::String
521510
end
522-
export UnusedImportData
511+
export AmbiguousSoftScopeData
523512

524513
struct UnreachableCodeData
525514
delete_range::Range
@@ -531,6 +520,23 @@ struct UnsortedImportData
531520
end
532521
export UnsortedImportData
533522

523+
struct UnusedArgumentData
524+
is_kwarg::Bool
525+
end
526+
export UnusedArgumentData
527+
528+
struct UnusedImportData
529+
delete_range::Range
530+
end
531+
export UnusedImportData
532+
533+
struct UnusedVariableData
534+
is_tuple_unpacking::Bool
535+
assignment_range::Union{Nothing,Range}
536+
lhs_eq_range::Union{Nothing,Range}
537+
end
538+
export UnusedVariableData
539+
534540
"""
535541
Represents a diagnostic, such as a compiler error or warning.
536542
Diagnostic objects are only valid in the scope of a resource.
@@ -588,7 +594,7 @@ Diagnostic objects are only valid in the scope of a resource.
588594
# Tags
589595
- since – 3.16.0
590596
"""
591-
data::Union{UnusedArgumentData, UnusedVariableData, UnusedImportData, UnreachableCodeData, UnsortedImportData, Nothing} = nothing
597+
data::Union{AmbiguousSoftScopeData, UnsortedImportData, UnreachableCodeData, UnusedArgumentData, UnusedImportData, UnusedVariableData, Nothing} = nothing
592598
end
593599

594600
# Command

docs/src/diagnostic.md

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ Here is a summary table of the diagnostics explained in this section:
115115
| [`lowering/unused-import`](@ref diagnostic/reference/lowering/unused-import) | `Information` | `JETLS/live` | Imported names that are never used |
116116
| [`lowering/unreachable-code`](@ref diagnostic/reference/lowering/unreachable-code) | `Information` | `JETLS/live` | Code after a block terminator that is never reached |
117117
| [`lowering/unsorted-import-names`](@ref diagnostic/reference/lowering/unsorted-import-names) | `Hint` | `JETLS/live` | Import/export names not sorted alphabetically |
118+
| [`lowering/ambiguous-soft-scope`](@ref diagnostic/reference/lowering/ambiguous-soft-scope) | `Warning` | `JETLS/live` | Assignment in soft scope shadows a global variable |
118119
| [`toplevel/error`](@ref diagnostic/reference/toplevel/error) | `Error` | `JETLS/save` | Errors during code loading |
119120
| [`toplevel/method-overwrite`](@ref diagnostic/reference/toplevel/method-overwrite) | `Warning` | `JETLS/save` | Method definitions that overwrite previous ones |
120121
| [`toplevel/abstract-field`](@ref diagnostic/reference/toplevel/abstract-field) | `Information` | `JETLS/save` | Struct fields with abstract types |
@@ -602,6 +603,86 @@ export bar, @foo # Names are not sorted alphabetically (JETLS lowering/unsorted
602603
[Julia's conventional maximum line length](https://docs.julialang.org/en/v1.14-dev/devdocs/contributing/formatting/#General-Formatting-Guidelines-for-Julia-code-contributions)),
603604
the code action wraps to multiple lines with 4-space continuation indent.
604605
606+
#### [Ambiguous soft scope (`lowering/ambiguous-soft-scope`)](@id diagnostic/reference/lowering/ambiguous-soft-scope)
607+
608+
**Default severity**: `Warning`
609+
610+
Reported when a variable is assigned inside a `for`, `while`, or
611+
`try`/`catch` block at the top level of a file, and a global variable
612+
with the same name already exists[^on_soft_scope]. This assignment is
613+
ambiguous because it behaves differently depending on where the code
614+
runs:
615+
- In the REPL or notebooks: assigns to the existing global
616+
- In a file: creates a new local variable, leaving the global
617+
unchanged
618+
619+
[^on_soft_scope]: See
620+
[On Soft Scope](https://docs.julialang.org/en/v1/manual/variables-and-scoping/#on-soft-scope)
621+
in the Julia manual.
622+
623+
Example ([A Common Confusion](https://docs.julialang.org/en/v1/manual/variables-and-scoping/#A-Common-Confusion-2479cb3548c466db) adapted from the Julia manual):
624+
625+
```@eval
626+
using Markdown
627+
628+
mktemp() do file, io
629+
code = """
630+
# Print the numbers 1 through 5
631+
global i = 0
632+
while i < 5
633+
i += 1 # Assignment to `i` in soft scope is ambiguous (JETLS lowering/ambiguous-soft-scope)
634+
# Variable `i` may be used before it is defined (JETLS lowering/undef-local-var)
635+
println(i)
636+
end
637+
"""
638+
write(io, code)
639+
close(io)
640+
err = IOBuffer()
641+
try
642+
run(pipeline(`$(Base.julia_cmd()) --startup-file=no $file`; stderr=err))
643+
catch
644+
end
645+
output = String(take!(err))
646+
lines = split(output, '\n')
647+
idx = findfirst(l -> startswith(l, "Stacktrace:"), lines)
648+
if idx !== nothing
649+
lines = lines[1:idx]
650+
end
651+
push!(lines, "...")
652+
output = join(lines, '\n')
653+
output = replace(output, file=>"ambiguous-scope.jl")
654+
655+
Markdown.parse("""
656+
> `ambiguous-scope.jl`
657+
``````julia
658+
$(code)
659+
``````
660+
661+
This diagnostic matches the warning that Julia itself emits at runtime.
662+
Running the example above as a file produces:
663+
> `julia ambiguous-scope.jl`
664+
``````
665+
$(output)
666+
``````
667+
""")
668+
end
669+
```
670+
671+
!!! note "Why is `lowering/undef-local-var` also reported?"
672+
Since `i += 1` desugars to `i = i + 1`, the new local `i` is read
673+
before being assigned, which also triggers
674+
[`lowering/undef-local-var`](@ref diagnostic/reference/lowering/undef-local-var)
675+
and causes the `UndefVarError` shown above at runtime.
676+
677+
!!! tip "Code actions available"
678+
Two quick fixes are offered: "Insert `global i` declaration" (preferred)
679+
to assign to the existing global, and "Insert `local i` declaration" to
680+
explicitly mark the variable as local and suppress the warning.
681+
682+
!!! note "Notebook mode"
683+
This diagnostic is suppressed for [notebooks](@ref notebook), where soft
684+
scope semantics are enabled (matching REPL behavior).
685+
605686
### [Top-level diagnostic (`toplevel/*`)](@id diagnostic/reference/toplevel)
606687
607688
Top-level diagnostics are reported by JETLS's full analysis feature (source:

src/code-action.jl

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ function handle_CodeActionRequest(
4141
unused_import_code_actions!(code_actions, uri, diagnostics)
4242
unreachable_code_actions!(code_actions, uri, diagnostics)
4343
sort_imports_code_actions!(code_actions, uri, diagnostics)
44+
ambiguous_soft_scope_code_actions!(code_actions, uri, diagnostics)
4445
return send(server,
4546
CodeActionResponse(;
4647
id = msg.id,
@@ -196,3 +197,36 @@ function sort_imports_code_actions!(
196197
end
197198
return code_actions
198199
end
200+
201+
function ambiguous_soft_scope_code_actions!(
202+
code_actions::Vector{Union{CodeAction,Command}}, uri::URI,
203+
diagnostics::Vector{Diagnostic}
204+
)
205+
for diagnostic in diagnostics
206+
diagnostic.code == LOWERING_AMBIGUOUS_SOFT_SCOPE_CODE || continue
207+
data = diagnostic.data
208+
data isa AmbiguousSoftScopeData || continue
209+
insert_pos = Position(; line = diagnostic.range.start.line, character = 0)
210+
insert_range = Range(; start = insert_pos, var"end" = insert_pos)
211+
push!(code_actions, CodeAction(;
212+
title = "Insert `global $(data.name)` declaration",
213+
kind = CodeActionKind.QuickFix,
214+
diagnostics = Diagnostic[diagnostic],
215+
isPreferred = true,
216+
edit = WorkspaceEdit(;
217+
changes = Dict{URI,Vector{TextEdit}}(
218+
uri => TextEdit[TextEdit(;
219+
range = insert_range,
220+
newText = data.indent * "global $(data.name)\n")]))))
221+
push!(code_actions, CodeAction(;
222+
title = "Insert `local $(data.name)` declaration",
223+
kind = CodeActionKind.QuickFix,
224+
diagnostics = Diagnostic[diagnostic],
225+
edit = WorkspaceEdit(;
226+
changes = Dict{URI,Vector{TextEdit}}(
227+
uri => TextEdit[TextEdit(;
228+
range = insert_range,
229+
newText = data.indent * "local $(data.name)\n")]))))
230+
end
231+
return code_actions
232+
end

src/diagnostic.jl

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -960,6 +960,38 @@ function find_capture_sites(
960960
return @somereal relatedInformation Some(nothing)
961961
end
962962

963+
function analyze_ambiguous_soft_scope!(
964+
diagnostics::Vector{Diagnostic}, fi::FileInfo, ctx3::JL.VariableAnalysisContext,
965+
reported::Set{LoweringDiagnosticKey}
966+
)
967+
for binfo in ctx3.bindings.info
968+
binfo.is_ambiguous_local || continue
969+
binfo.is_internal && continue
970+
bn = binfo.name
971+
startswith(bn, '#') && continue
972+
provs = JS.flattened_provenance(JL.binding_ex(ctx3, binfo.id))
973+
is_from_user_ast(provs) || continue
974+
prov = last(provs)
975+
range = jsobj_to_range(prov, fi)
976+
key = LoweringDiagnosticKey(range, :ambiguous, bn)
977+
key in reported ? continue : push!(reported, key)
978+
indent = get_line_indent(fi, range.start.line)
979+
push!(diagnostics, Diagnostic(;
980+
range,
981+
severity = DiagnosticSeverity.Warning,
982+
message = "Assignment to `$bn` in soft scope is ambiguous " *
983+
"because a global variable by the same name exists: " *
984+
"`$bn` will be treated as a new local. " *
985+
"Disambiguate by using `local $bn` to suppress this " *
986+
"warning or `global $bn` to assign to the existing " *
987+
"global variable.",
988+
source = DIAGNOSTIC_SOURCE_LIVE,
989+
code = LOWERING_AMBIGUOUS_SOFT_SCOPE_CODE,
990+
codeDescription = diagnostic_code_description(LOWERING_AMBIGUOUS_SOFT_SCOPE_CODE),
991+
data = AmbiguousSoftScopeData(bn, indent)))
992+
end
993+
end
994+
963995
const SORT_IMPORTS_MAX_LINE_LENGTH = 92
964996
const SORT_IMPORTS_INDENT = " "
965997

@@ -1196,6 +1228,8 @@ function analyze_lowered_code!(
11961228

11971229
has_implicit_args = ismacro[] || is_generated0(st0)
11981230

1231+
analyze_ambiguous_soft_scope!(diagnostics, fi, ctx3, reported)
1232+
11991233
analyze_unused_bindings!(
12001234
diagnostics, fi, st0, ctx3, binding_occurrences, has_implicit_args, reported,
12011235
kwarg_type_names, kwarg_locations;
@@ -1218,7 +1252,9 @@ end
12181252

12191253
function lowering_diagnostics!(
12201254
diagnostics::Vector{Diagnostic}, uri::URI, fi::FileInfo, mod::Module, st0::JS.SyntaxTree;
1221-
skip_analysis_requiring_context::Bool = false, kwargs...
1255+
skip_analysis_requiring_context::Bool = false,
1256+
soft_scope::Bool = false,
1257+
kwargs...
12221258
)
12231259
@assert JS.kind(st0) JS.KSet"toplevel module"
12241260

@@ -1227,7 +1263,8 @@ function lowering_diagnostics!(
12271263
(st0, _) = desugar_main_macrocall(st0)
12281264
world = Base.get_world_counter()
12291265
res = try
1230-
jl_lower_for_scope_resolution(mod, st0, world; recover_from_macro_errors=false, convert_closures=true)
1266+
jl_lower_for_scope_resolution(mod, st0, world;
1267+
recover_from_macro_errors=false, convert_closures=true, soft_scope)
12311268
catch err
12321269
if err isa JL.LoweringError
12331270
if !err.internal
@@ -1488,12 +1525,14 @@ function toplevel_lowering_diagnostics(
14881525
st0_top = build_syntax_tree(file_info)
14891526
skip_analysis_requiring_context = !has_analyzed_context(server.state, uri; lookup_func)
14901527
allow_unused_underscore = get_config(server, :diagnostic, :allow_unused_underscore)
1528+
soft_scope = is_notebook_cell_uri(server.state, uri)
14911529
iterate_toplevel_tree(st0_top) do st0::JS.SyntaxTree
14921530
is_cancelled(cancel_flag) && return traversal_terminator
14931531
pos = offset_to_xy(file_info, JS.first_byte(st0))
14941532
(; mod, analyzer, postprocessor) = get_context_info(server.state, uri, pos; lookup_func)
14951533
lowering_diagnostics!(diagnostics, uri, file_info, mod, st0;
1496-
skip_analysis_requiring_context, allow_unused_underscore, analyzer, postprocessor)
1534+
skip_analysis_requiring_context, allow_unused_underscore, soft_scope,
1535+
analyzer, postprocessor)
14971536
end
14981537

14991538
if !skip_analysis_requiring_context

src/notebook.jl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,8 @@ end
227227

228228
is_notebook_uri(state::ServerState, uri::URI) = haskey(load(state.notebook_cache), uri)
229229

230+
is_notebook_cell_uri(state::ServerState, uri::URI) = haskey(load(state.cell_to_notebook), uri)
231+
230232
function map_notebook_diagnostics!(uri2diagnostics::URI2Diagnostics, state::ServerState)
231233
notebook_uris_to_delete = URI[]
232234
for uri in keys(uri2diagnostics)

src/types.jl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,7 @@ const LOWERING_CAPTURED_BOXED_VARIABLE_CODE = "lowering/captured-boxed-variable"
394394
const LOWERING_UNSORTED_IMPORT_NAMES_CODE = "lowering/unsorted-import-names"
395395
const LOWERING_UNUSED_IMPORT_CODE = "lowering/unused-import"
396396
const LOWERING_UNREACHABLE_CODE = "lowering/unreachable-code"
397+
const LOWERING_AMBIGUOUS_SOFT_SCOPE_CODE = "lowering/ambiguous-soft-scope"
397398
const TOPLEVEL_ERROR_CODE = "toplevel/error"
398399
const TOPLEVEL_METHOD_OVERWRITE_CODE = "toplevel/method-overwrite"
399400
const TOPLEVEL_ABSTRACT_FIELD_CODE = "toplevel/abstract-field"
@@ -417,6 +418,7 @@ const ALL_DIAGNOSTIC_CODES = Set{String}(String[
417418
LOWERING_UNSORTED_IMPORT_NAMES_CODE,
418419
LOWERING_UNUSED_IMPORT_CODE,
419420
LOWERING_UNREACHABLE_CODE,
421+
LOWERING_AMBIGUOUS_SOFT_SCOPE_CODE,
420422
TOPLEVEL_ERROR_CODE,
421423
TOPLEVEL_METHOD_OVERWRITE_CODE,
422424
TOPLEVEL_ABSTRACT_FIELD_CODE,

src/utils/binding.jl

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,21 @@ function is_relevant(ctx3::JL.AbstractLoweringContext,
2929
|| cursor > start)
3030
end
3131

32+
function insert_softscope_marker(st1::JS.SyntaxTree)
33+
g = JS.syntax_graph(st1)
34+
marker = JS.newleaf(g, st1, JS.K"softscope")
35+
children = JS.SyntaxList(g)
36+
push!(children, marker, st1)
37+
return JS.newnode(g, st1, JS.K"block", children)
38+
end
39+
3240
"""
3341
jl_lower_for_scope_resolution(
3442
mod::Module, st0::JS.SyntaxTree;
3543
trim_error_nodes::Bool = true,
3644
recover_from_macro_errors::Bool = true,
3745
convert_closures::Bool = false,
46+
soft_scope::Bool = false,
3847
) -> (; st0, st1, st2, st3, ctx3)
3948
4049
Perform the first three passes of lowering.
@@ -43,6 +52,11 @@ Depending on keyword arguments, also attempt to handle junk input in the ways a
4352
- `recover_from_macro_errors`: If errors occur during macro expansion, trim macrocalls and retry.
4453
This may happen for several reasons; the analyzer may not have picked up the
4554
macro definition yet, or the user could be working on the macrocall.
55+
- `convert_closures`: Also run the closure conversion pass (pass 4),
56+
returning `ctx4` and `st4` in addition to the scope resolution results.
57+
- `soft_scope`: Enable soft scope semantics (REPL/notebook behavior)
58+
where assignments in `for`/`while`/`try` blocks at the top level assign to
59+
existing globals instead of creating new locals.
4660
4761
Throw if lowering fails otherwise.
4862
@@ -53,6 +67,7 @@ function jl_lower_for_scope_resolution(
5367
trim_error_nodes::Bool = true,
5468
recover_from_macro_errors::Bool = true,
5569
convert_closures::Bool = false,
70+
soft_scope::Bool = false,
5671
)
5772
if trim_error_nodes
5873
st0 = without_kinds(st0, JS.KSet"error")
@@ -67,14 +82,18 @@ function jl_lower_for_scope_resolution(
6782
st0 = remove_macrocalls(st0)
6883
JL.expand_forms_1(mod, st0, true, world)
6984
end
70-
return _jl_lower_for_scope_resolution(ctx1, st0, st1; convert_closures)
85+
return _jl_lower_for_scope_resolution(ctx1, st0, st1; convert_closures, soft_scope)
7186
end
7287

7388
function _jl_lower_for_scope_resolution(
7489
ctx1::JL.MacroExpansionContext, st0::JS.SyntaxTree, st1::JS.SyntaxTree;
75-
convert_closures::Bool = false
90+
convert_closures::Bool = false,
91+
soft_scope::Bool = false,
7692
)
7793
ctx2, st2 = JL.expand_forms_2(ctx1, st1)
94+
if soft_scope
95+
st2 = insert_softscope_marker(st2)
96+
end
7897
ctx3, st3 = JL.resolve_scopes(ctx2, st2)
7998
convert_closures || return (; st0, st1, st2, st3, ctx3)
8099
ctx4, st4 = JL.convert_closures(ctx3, st3)

0 commit comments

Comments
 (0)