Fix violations found by running JuliaCheck on itself

checks/AvoidContainersWithAbstractTypes.jl

@@ -38,28 +38,30 @@ const ABSTRACT_NUMBER_TYPES = Set([
 ])
 
 struct Check<:Analysis.Check end
-id(::Check) = "avoid-containers-with-abstract-types"
-severity(::Check) = 6
-synopsis(::Check) = "Avoid containers with abstract types."
+Analysis.id(::Check) = "avoid-containers-with-abstract-types"
+Analysis.severity(::Check) = 6
+Analysis.synopsis(::Check) = "Avoid containers with abstract types."
 
-function init(this::Check, ctxt::AnalysisContext)::Nothing
-    register_syntaxnode_action(ctxt, is_container, n -> check(this, ctxt, n))
+function Analysis.init(this::Check, ctxt::AnalysisContext)::Nothing
+    register_syntaxnode_action(ctxt, _is_container, n -> _check(this, ctxt, n))
     return nothing
 end
 
-# Structure of a typical node we want to check here:
-# (= num_vector (ref Real 1.0 2 3))
-# We want to check the type right-hand side of the assignment.
-# For some invocations, this is also wrapped inside a call (eg. list comprehensions)
-function is_container(node::SyntaxNode)::Bool
+#=
+Structure of a typical node we want to check here:
+(= num_vector (ref Real 1.0 2 3))
+We want to check the type right-hand side of the assignment.
+For some invocations, this is also wrapped inside a call (eg. list comprehensions)
+=#
+function _is_container(node::SyntaxNode)::Bool
     if !is_assignment(node) || numchildren(node) < 2
         return false
     end
     rhs = children(node)[2]
     return !is_leaf(rhs) && kind(rhs) in KSet"ref call curly"
 end
 
-function check(this::Check, ctxt::AnalysisContext, node::SyntaxNode)::Nothing
+function _check(this::Check, ctxt::AnalysisContext, node::SyntaxNode)::Nothing
     assignment_rhs = children(node)[2]
     id_type_node = _get_identifier_node_to_check(assignment_rhs)
     if !isnothing(id_type_node)
@@ -76,11 +78,13 @@ function check(this::Check, ctxt::AnalysisContext, node::SyntaxNode)::Nothing
     return nothing
 end
 
-# Curly braces notations get translated like this:
-# - Array{Number}[] => (curly Array Number)
-# - Array{Array}{Number}[] => (curly Array (curly Array Number))
-# As such, to find the type of multidimensional arrays, it's convenient to be able
-# to walk down the tree until an identifier is found.
+#=
+Curly braces notations get translated like this:
+- Array{Number}[] => (curly Array Number)
+- Array{Array}{Number}[] => (curly Array (curly Array Number))
+As such, to find the type of multidimensional arrays, it's convenient to be able
+to walk down the tree until an identifier is found.
+=#
 function _get_identifier_node_to_check(node::SyntaxNode)::NullableNode
     while _search_further(node)
         node = _get_next_search_node(node)

checks/AvoidCreatingEmptyArraysAndVectors.jl

@@ -6,16 +6,16 @@ using ...SymbolTable: node_is_declaration_of_variable
 include("_common.jl")
 
 struct Check<:Analysis.Check end
-id(::Check) = "avoid-creating-empty-arrays-and-vectors"
-severity(::Check) = 8
-synopsis(::Check) = "Avoid resizing arrays after initialization."
+Analysis.id(::Check) = "avoid-creating-empty-arrays-and-vectors"
+Analysis.severity(::Check) = 8
+Analysis.synopsis(::Check) = "Avoid resizing arrays after initialization."
 
-function init(this::Check, ctxt::AnalysisContext)::Nothing
-    register_syntaxnode_action(ctxt, is_assignment, n -> check(this, ctxt, n))
+function Analysis.init(this::Check, ctxt::AnalysisContext)::Nothing
+    register_syntaxnode_action(ctxt, is_assignment, n -> _check(this, ctxt, n))
     return nothing
 end
 
-function check(this::Check, ctxt::AnalysisContext, assignment_node::SyntaxNode)::Nothing
+function _check(this::Check, ctxt::AnalysisContext, assignment_node::SyntaxNode)::Nothing
     if ! node_is_declaration_of_variable(ctxt.symboltable, first(children(assignment_node)))
         return
     end

checks/AvoidExtraneousWhitespaceBetweenOpenAndCloseCharacters.jl

@@ -6,9 +6,11 @@ using ...Properties: is_toplevel
 include("_common.jl")
 
 struct Check<:Analysis.Check end
-id(::Check) = "avoid-extraneous-whitespace-between-open-and-close-characters"
-severity(::Check) = 7
-synopsis(::Check) = "Avoid extraneous whitespace inside parentheses, square brackets or braces."
+Analysis.id(::Check) = "avoid-extraneous-whitespace-between-open-and-close-characters"
+Analysis.severity(::Check) = 7
+function Analysis.synopsis(::Check)::String
+    return "Avoid extraneous whitespace inside parentheses, square brackets or braces."
+end
 
 """
 Syntax node types for which whitespace should be checked.
@@ -100,7 +102,7 @@ function _check(this::Check, ctxt::AnalysisContext, sf::SourceFile)::Nothing
             location_msg = " before '$next_leaf_text'"
         else
             expected_spaces = 1 # Exactly one space between elements
-            location_msg = " between '$prev_leaf_text' and '$next_leaf_text'" 
+            location_msg = " between '$prev_leaf_text' and '$next_leaf_text'"
         end
 
         if !isnothing(expected_spaces) && length(cur.range) != expected_spaces
@@ -111,7 +113,7 @@ function _check(this::Check, ctxt::AnalysisContext, sf::SourceFile)::Nothing
     return nothing
 end
 
-function init(this::Check, ctxt::AnalysisContext)::Nothing
+function Analysis.init(this::Check, ctxt::AnalysisContext)::Nothing
     register_syntaxnode_action(ctxt, is_toplevel, root -> _check(this, ctxt, root.source))
     return nothing
 end

checks/AvoidGlobalVariables.jl

@@ -1,20 +1,20 @@
 module AvoidGlobalVariables
 
 using ...Properties: is_global_decl, is_constant, find_lhs_of_kind
-using ...SyntaxNodeHelpers
 using ...SymbolTable
+using ...SyntaxNodeHelpers
 
 include("_common.jl")
 
 struct Check<:Analysis.Check
     already_reported::Set{SyntaxNode}
     Check() = new(Set{SyntaxNode}())
 end
-id(::Check) = "avoid-global-variables"
-severity(::Check) = 3
-synopsis(::Check) = "Avoid global variables when possible"
+Analysis.id(::Check) = "avoid-global-variables"
+Analysis.severity(::Check) = 3
+Analysis.synopsis(::Check) = "Avoid global variables when possible"
 
-function init(this::Check, ctxt::AnalysisContext)
+function Analysis.init(this::Check, ctxt::AnalysisContext)::Nothing
     register_syntaxnode_action(ctxt, is_global_decl, node -> begin
         id = find_lhs_of_kind(K"Identifier", node)
         if isnothing(id)
@@ -35,6 +35,7 @@ function init(this::Check, ctxt::AnalysisContext)
         report_violation(ctxt, this, id, synopsis(this))
         return nothing
     end)
+    return nothing
 end
 
 end # module AvoidGlobalVariables

checks/AvoidHardCodedNumbers.jl

@@ -4,32 +4,60 @@ using ...Properties: get_number, is_constant, is_global_decl, is_literal_number
 
 include("_common.jl")
 
+""" Positive powers of 10 (up till 10^18) """
+const POWERS_OF_TEN_POSITIVE = Set{Int64}([10^i for i in 1:18])
+
+""" Negative powers of 10 (up till -10^18) """
+const POWERS_OF_TEN_NEGATIVE = Set{Int64}([-i for i in POWERS_OF_TEN_POSITIVE])
+
+""" Positive and negative powers of 10 """
+const POWERS_OF_TEN = POWERS_OF_TEN_POSITIVE ∪ POWERS_OF_TEN_NEGATIVE
+
+""" Positive powers of 2 (up till 2^20) """
+const POWERS_OF_TWO = Set{Int64}([2^i for i in 1:20])
+
+""" Integers with special meaning, such as number of seconds, degrees, etc . """
+const SOME_SPECIAL_INTS = Set{Int64}([0, 1,
+                                        60, # minutes, seconds
+                                        90, 180, 270, 360   # degrees
+                                    ])
+
+""" Integers that are not considered magical and may appear as constants. """
+const KNOWN_INTS = SOME_SPECIAL_INTS ∪ POWERS_OF_TEN ∪ POWERS_OF_TWO
+
+""" Floats that are not considered magical and may appear as constants. """
+const KNOWN_FLOATS = Set{Float64}([0.1, 0.01, 0.001, 0.0001, 0.5]) ∪
+                    Set{Float64}(convert.(Float64, POWERS_OF_TEN)) ∪
+                    Set{Float64}(convert.(Float64, SOME_SPECIAL_INTS))
+
 struct Check<:Analysis.Check
-    seen_before
+    seen_before::Set{Number}
 
     # FIXME Fine for integers but, for floats, we should
     # probably use a tolerance to compare them.
     Check() = new(Set{Number}())
 end
-id(::Check) = "avoid-hard-coded-numbers"
-severity(::Check) = 3
-synopsis(::Check) = "Avoid hard-coded numbers"
+Analysis.id(::Check) = "avoid-hard-coded-numbers"
+Analysis.severity(::Check) = 3
+Analysis.synopsis(::Check) = "Avoid hard-coded numbers"
 
-function init(this::Check, ctxt::AnalysisContext)
-    register_syntaxnode_action(ctxt, is_literal_number, n -> check(this, ctxt, n))
+function Analysis.init(this::Check, ctxt::AnalysisContext)::Nothing
+    register_syntaxnode_action(ctxt, is_literal_number, n -> _check(this, ctxt, n))
+    return nothing
 end
 
 # Also FIXME: should I use all the 64 bits versions of the types?
-function check(this::Check, ctxt::AnalysisContext, node::SyntaxNode)
+function _check(this::Check, ctxt::AnalysisContext, node::SyntaxNode)::Nothing
     @assert is_literal_number(node) "Expected a node with a literal number, got $(kind(node))"
-    if !is_const_declaration(node) && !in_array_assignment(node) && is_magic_number(node)
+    if !_is_const_declaration(node) && !_in_array_assignment(node) && _is_magic_number(node)
         n = get_number(node)
         if n ∈ this.seen_before
             report_violation(ctxt, this, node, "Hard-coded number '$n' should be a const variable.")
         else
             push!(this.seen_before, n)
         end
     end
+    return nothing
 end
 
 """
@@ -39,43 +67,28 @@ Check if the literal number is part of a constant declaration.
 
 To that end, we climb up the tree until we find a constant declaration, or the root.
 """
-function is_const_declaration(node::SyntaxNode)::Bool
+function _is_const_declaration(node::SyntaxNode)::Bool
     x = node
     while !(isnothing(x) || is_constant(x))
         x = x.parent
     end
     return !isnothing(x)
 end
 
-function in_array_assignment(node::SyntaxNode)::Bool
+function _in_array_assignment(node::SyntaxNode)::Bool
     p = node.parent
     return !isnothing(p) && kind(p) == K"vect"
 end
 
 # TODO Add (unit?) tests
 
-## Magic numbers
-# Integers
-const POWERS_OF_TEN_POSITIVE = Set{Int64}([10^i for i in 1:18])
-const POWERS_OF_TEN_NEGATIVE = Set{Int64}([-i for i in POWERS_OF_TEN_POSITIVE])
-const POWERS_OF_TEN = POWERS_OF_TEN_POSITIVE ∪ POWERS_OF_TEN_NEGATIVE
-const POWERS_OF_TWO = Set{Int64}([2^i for i in 1:20])
-const SOME_SPECIAL_INTS = Set{Int64}([0, 1,
-                                        60, # minutes, seconds
-                                        90, 180, 270, 360   # degrees
-                                    ])
-const KNOWN_INTS = SOME_SPECIAL_INTS ∪ POWERS_OF_TEN ∪ POWERS_OF_TWO
-# Floats
-const KNOWN_FLOATS = Set{Float64}([0.1, 0.01, 0.001, 0.0001, 0.5]) ∪
-                    Set{Float64}(convert.(Float64, POWERS_OF_TEN)) ∪
-                    Set{Float64}(convert.(Float64, SOME_SPECIAL_INTS))
 """
     is_magic_number(node::SyntaxNode)::Bool
 
 Check if the given literal is a magic number, i.e., it is not a "usual number",
 i.e., one usually found in initializations.
 """
-function is_magic_number(node::SyntaxNode)::Bool
+function _is_magic_number(node::SyntaxNode)::Bool
     n = get_number(node)
     return !isnothing(n) && (
                 kind(node) == K"Float" ? n ∉ KNOWN_FLOATS : n ∉ KNOWN_INTS

checks/ConsistentLineEndings.jl

@@ -6,12 +6,12 @@ using ...Properties: is_toplevel
 include("_common.jl")
 
 struct Check<:Analysis.Check end
-id(::Check) = "consistent-line-endings"
-severity(::Check) = 7
-synopsis(::Check) = "Make sure that the line endings are consistent within a file"
+Analysis.id(::Check) = "consistent-line-endings"
+Analysis.severity(::Check) = 7
+Analysis.synopsis(::Check) = "Make sure that the line endings are consistent within a file"
 
 
-function init(this::Check, ctxt::AnalysisContext)::Nothing
+function Analysis.init(this::Check, ctxt::AnalysisContext)::Nothing
     register_syntaxnode_action(ctxt, is_toplevel, n -> _check(this, ctxt, n))
     return nothing
 end

checks/DoNotChangeGeneratedIndices.jl

@@ -1,47 +1,49 @@
 module DoNotChangeGeneratedIndices
 
 using ...Properties: first_child, get_assignee, get_iteration_parts,
-                     is_assignment, is_flow_cntrl, is_range
+                    is_assignment, is_flow_cntrl, is_range
 
 include("_common.jl")
-                     
+
 struct Check<:Analysis.Check end
-id(::Check) = "do-not-change-generated-indices"
-severity(::Check) = 5
-synopsis(::Check) = "Do not change generated indices"
+Analysis.id(::Check) = "do-not-change-generated-indices"
+Analysis.severity(::Check) = 5
+Analysis.synopsis(::Check) = "Do not change generated indices"
 
-function init(this::Check, ctxt::AnalysisContext)
-    register_syntaxnode_action(ctxt, n -> kind(n) == K"for", n -> checkForLoop(this, ctxt, n))
+function Analysis.init(this::Check, ctxt::AnalysisContext)::Nothing
+    register_syntaxnode_action(ctxt, n -> kind(n) == K"for", n -> _check_for_loop(this, ctxt, n))
+    return nothing
 end
 
-function checkForLoop(this::Check, ctxt::AnalysisContext, for_loop::SyntaxNode)
+function _check_for_loop(this::Check, ctxt::AnalysisContext, for_loop::SyntaxNode)::Nothing
     loop_var, iter_expr = get_iteration_parts(for_loop)
     if isnothing(loop_var) || isnothing(iter_expr)
         return nothing
     end
-    var_name = loop_var_to_string(loop_var)
+    var_name = _loop_var_to_string(loop_var)
     if is_range(iter_expr) || (
         kind(iter_expr) == K"call" &&
         kind(first_child(iter_expr)) == K"Identifier" &&
         string(first_child(iter_expr)) ∈ ["eachindex", "enumerate", "axes"]
-       )
+        )
         # Look into the loop's body to see if `loop_var` is modified.
         @assert numchildren(for_loop) == 2 &&
                 kind(children(for_loop)[2]) == K"block" "An empty loop or what? $for_loop"
         body = children(for_loop)[2]
-        frisk_for_modification(this, ctxt, body, var_name)
+        _frisk_for_modification(this, ctxt, body, var_name)
     end
+    return nothing
 end
 
-function loop_var_to_string(var::SyntaxNode)
+function _loop_var_to_string(var::SyntaxNode)::String
     x = var
     if kind(x) == K"tuple" x = first_child(x) end
     if kind(x) == K"Identifier" return string(x) end
     @debug "Can't find identifier in loop variable" var
     return ""
 end
 
-function frisk_for_modification(this::Check, ctxt::AnalysisContext, body::SyntaxNode, var_name::String)::Nothing
+function _frisk_for_modification(this::Check, ctxt::AnalysisContext, body::SyntaxNode, var_name::String)::Nothing
     for expr in children(body)
         if is_assignment(expr)
             lhs_node, lhs_str = get_assignee(expr)
@@ -52,7 +54,7 @@ function frisk_for_modification(this::Check, ctxt::AnalysisContext, body::Syntax
         elseif is_flow_cntrl(expr)
             next_victim = findfirst(x -> kind(x) == K"block", children(expr))
             if ! isnothing(next_victim)
-                frisk_for_modification(this, ctxt, children(expr)[next_victim], var_name)
+                _frisk_for_modification(this, ctxt, children(expr)[next_victim], var_name)
             end
         end
     end