Skip to content

RM-37765: Fix FP for document-constants for type aliases #116

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.

Sign up for GitHub

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

Merged
reniers-tiobe merged 2 commits into from
Jan 23, 2026
Merged

RM-37765: Fix FP for document-constants for type aliases #116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1004b92
Fix FP for `document-constants`
reniers-tiobe Jan 23, 2026
18bfab5
Merge branch 'main' into 37765-fix-fp-document-constants
reniers-tiobe Jan 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 16 additions & 21 deletions checks/DocumentConstants.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
module DocumentConstants

using ...Properties: find_lhs_of_kind, haschildren, is_constant
using ...SyntaxNodeHelpers: find_descendants

include("_common.jl")

Expand All @@ -10,31 +11,25 @@ severity(::Check) = 7
synopsis(::Check) = "Constants must have a docstring"

function init(this::Check, ctxt::AnalysisContext)
register_syntaxnode_action(ctxt, is_constant, n -> check(this, ctxt, n))
register_syntaxnode_action(ctxt, n -> kind(n) == K"const", n -> check(this, ctxt, n))
end

function check(this::Check, ctxt::AnalysisContext, const_node::SyntaxNode)
if kind(const_node) == K"global"
if haschildren(const_node)
const_node = children(const_node)[1]
else
@debug "A global node without children:" const_node
return nothing
end
end
@assert kind(const_node) == K"const" "Expected a [const] const_node, got $(kind(const_node))."
if haschildren(const_node) && kind(children(const_node)[1]) == K"="
# This is a constant value declaration.
assignment = children(const_node)[1]
if haschildren(assignment)
if kind(const_node.parent) != K"doc"
const_id = find_lhs_of_kind(K"Identifier", const_node)
report_violation(ctxt, this, const_node,
"Const value $(string(const_id)) has no docstring."
)
end
else
@debug "An assignment without children:" assignment

if kind(const_node.parent) == K"doc"
Copy link
Contributor

@brouwers-tiobe brouwers-tiobe Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the doc node is not necessarily the direct parent; for example I'm getting a FP on the following:

"doc"
global const n = 1

as global would be in between const and doc:

[toplevel]
  [doc]
    [string]
      "doc"                              :: String
    [global]
      [const]
        [=]
          n                              :: Identifier
          1                              :: Integer

However, I'm not sure when this construction would reasonably be used (it violates no-global-keyword-in-global-scope as well)

Copy link
Contributor Author

@reniers-tiobe reniers-tiobe Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find!

return nothing # Nothing to report: there is a docstring already
end

# Find first (typically only) assignment underneath [const] node
assignment = first(find_descendants(n -> kind(n) == K"=", const_node, true))
if ! isnothing(assignment) && length(children(assignment)) >= 2
rhs = children(assignment)[2]
if kind(rhs) != K"curly" # RM-37765: ignore Union and other parameterized type
const_id = find_lhs_of_kind(K"Identifier", const_node)
report_violation(ctxt, this, const_node,
"Const value '$(string(const_id))' has no docstring"
)
end
end
end
Expand Down
6 changes: 6 additions & 0 deletions test/res/DocumentConstants.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ struct MyType
x::Int
end
const UNDOCUMENTED_GLOBAL_ITEM = MyType(3)
const myvar = {}
const MyVector = Vector{Float64}(undef, 3)

# Good
"MAG_THRESHOLD is dimensionless. A value of 0 means no expansion."
Expand All @@ -17,3 +19,7 @@ const THE_ANSWER = 42

"Cierto!"
const E_VERO = true

# RM-37765: No docstring needed for 'type aliases'
const AnyTree = Union{SyntaxNode, GreenNode}
const MyTuple = Tuple{SyntaxNode, GreenNode}
19 changes: 15 additions & 4 deletions test/res/DocumentConstants.val
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,31 @@

>> Processing file 'DocumentConstants.jl'...

DocumentConstants.jl(3, 1):
const MAGTHRESHOLD::Float64 = 3.00 # In ppm
└────────────────────────────────┘ ── Const value MAGTHRESHOLD has no docstring.
└────────────────────────────────┘ ── Const value 'MAGTHRESHOLD' has no docstring.
Constants must have a docstring.
Rule: document-constants. Severity: 7

DocumentConstants.jl(4, 1):
const THEANSWER = 42
└──────────────────┘ ── Const value THEANSWER has no docstring.
└──────────────────┘ ── Const value 'THEANSWER' has no docstring.
Constants must have a docstring.
Rule: document-constants. Severity: 7

DocumentConstants.jl(9, 1):
const UNDOCUMENTED_GLOBAL_ITEM = MyType(3)
└────────────────────────────────────────┘ ── Const value UNDOCUMENTED_GLOBAL_ITEM has no docstring.
└────────────────────────────────────────┘ ── Const value 'UNDOCUMENTED_GLOBAL_ITEM' has no docstring.
Constants must have a docstring.
Rule: document-constants. Severity: 7

DocumentConstants.jl(10, 1):
const myvar = {}
└──────────────┘ ── Const value 'myvar' has no docstring.
Constants must have a docstring.
Rule: document-constants. Severity: 7

DocumentConstants.jl(11, 1):
const MyVector = Vector{Float64}(undef, 3)
└────────────────────────────────────────┘ ── Const value 'MyVector' has no docstring.
Constants must have a docstring.
Rule: document-constants. Severity: 7