Unreachable branches

src/linting/checks.jl

@@ -141,7 +141,10 @@ function check_all(x::EXPR, opts::LintOptions, env::ExternalEnv, markers::Dict{S
     end
 
     if headof(x) === :macrocall
-        markers[:macrocall] = fetch_value(x, :IDENTIFIER)
+        id = fetch_value(x, :IDENTIFIER)
+        if !isnothing(id)
+            markers[:macrocall] = id
+        end
     end
 
     # Do checks

src/linting/extended_checks.jl

@@ -2,15 +2,23 @@
 #################################################################################
 # UTILITY FUNCTIONS
 #################################################################################
+function is_named_hole_variable(x::CSTParser.EXPR)
+    return x.head == :IDENTIFIER &&
+            startswith(x.val, "hole_variable") &&
+            x.val != "hole_variable_star" &&
+            length(x.val) > length("hole_variable")
+end
+
 function is_hole_variable(x::CSTParser.EXPR)
-    return x.head == :IDENTIFIER && x.val in ["hole_variable", "hole_variable_star"]
+    return x.head == :IDENTIFIER && startswith(x.val, "hole_variable")
 end
 
 function is_hole_variable_star(x::CSTParser.EXPR)
     return x.head == :IDENTIFIER && x.val == "hole_variable_star"
 end
 
 comp(x, y) = x == y
+raw_comp(x, y, named_variable_holes) = x == y
 
 struct BothCannotHaveStarException <: Exception
     msg::String
@@ -21,7 +29,8 @@ function comp_value(x::String, y::String)
     is_there_any_star_marker = contains(x, "QQQ") || contains(y, "QQQ")
     !is_there_any_star_marker && return x == y
 
-    contains(x, "QQQ") && contains(y, "QQQ") && throw(BothCannotHaveStarException("Cannot both $x and $y have a star marker"))
+    contains(x, "QQQ") && contains(y, "QQQ") &&
+        throw(BothCannotHaveStarException("Cannot both $x and $y have a star marker"))
     if contains(x, "QQQ")
         reg_exp = Regex(replace(x, "QQQ" => ".*"))
         return !isnothing(match(reg_exp, y))
@@ -31,16 +40,32 @@ function comp_value(x::String, y::String)
     end
 end
 
-function comp(x::CSTParser.EXPR, y::CSTParser.EXPR)
+function raw_comp(
+    x::CSTParser.EXPR,
+    y::CSTParser.EXPR,
+    named_variable_holes::Vector
+)
+    # @info "debug:" x y
+    # isdefined(Main, :Infiltrator) && Main.infiltrate(@__MODULE__, Base.@locals, @__FILE__, @__LINE__)
+
+    # If we bump into some named hole variables, then we record it.
+    if is_named_hole_variable(x)
+        push!(named_variable_holes, (x.val, y))
+    end
+    if is_named_hole_variable(y)
+        push!(named_variable_holes, (y.val, x))
+    end
+
+    # If one of element to be compared is a hole, then we have a match!
     (is_hole_variable(x) || is_hole_variable(y)) && return true
 
-    result = comp(x.head, y.head) && comp_value(x.val, y.val)
+    result = raw_comp(x.head, y.head, named_variable_holes) && comp_value(x.val, y.val)
     !result && return false
 
     min_length = min(length(x), length(y))
 
     for i in 1:min_length
-        comp(x[i], y[i]) || return false
+        raw_comp(x[i], y[i], named_variable_holes) || return false
         (is_hole_variable_star(x[i]) || is_hole_variable_star(y[i])) && return true
     end
 
@@ -57,6 +82,44 @@ function comp(x::CSTParser.EXPR, y::CSTParser.EXPR)
     return false
 end
 
+function comp(x::CSTParser.EXPR, y::CSTParser.EXPR)
+    named_variable_holes = Vector()
+    result = raw_comp(x, y, named_variable_holes)
+
+    # If there is no or only one named variable hole, then we can exit
+    length(named_variable_holes) <= 1 && return result
+
+    all_hole_names = Set(first.(named_variable_holes))
+    hole_names_to_values = Dict{String, CSTParser.EXPR}()
+    # Else, we need to check that values under a unique named hole is the same
+    for k in all_hole_names
+        # Retrieve all the value for the named hole k
+        relevant = filter(tp->first(tp) == k, named_variable_holes)
+        relevant = map(tp->tp[2], relevant)
+
+        # If there are more than 1 value for a given named hole k, then there is no match.
+        first_relevant = relevant[1]
+        all_others = relevant[2:end]
+        all(r -> comp(first_relevant, r), all_others) || return false
+
+        hole_names_to_values[k] = first_relevant
+    end
+
+    # Utility functions
+    remove!(a, item) = deleteat!(a, findall(x->x==item, a))
+    remove(a, item) = deleteat!(copy(a), findall(x->x==item, a))
+
+    # At this point, we know that all the values for each named hole are the same.
+    # We now need to check if values for each named holes are different.
+    # If some values for two different named holes are the same, then there is no match
+    nh_values = collect(values(hole_names_to_values))
+    for v in nh_values
+        all_to_check = remove(nh_values, v)
+        any(k -> comp(k, v), all_to_check) && return false
+    end
+    return true
+end
+
 #################################################################################
 # EXTENDED LINT RULES
 #################################################################################
@@ -97,6 +160,7 @@ struct HasKey_Extension <: ExtendedRule end
 struct Equal_Extension <: ExtendedRule end
 struct Uv_Extension <: ExtendedRule end
 struct Splatting_Extension <: ExtendedRule end
+struct UnreachableBranch_Extension <: ExtendedRule end
 
 
 const all_extended_rule_types = Ref{Any}(InteractiveUtils.subtypes(ExtendedRule))
@@ -297,3 +361,24 @@ function check(::Splatting_Extension, x::EXPR)
         "hole_variable([hole_variable(hole_variable_star) for hole_variable in hole_variable]...)",
         "Splatting (`...`) must not be used with dynamically sized containers. This may result in performance degradation.")
 end
+
+function check(::UnreachableBranch_Extension, x::EXPR)
+    generic_check(
+        x,
+        "if hole_variableA \
+            hole_variable \
+         elseif hole_variableA \
+            hole_variable \
+         end",
+        "Unreachable branch.")
+    generic_check(
+        x,
+        "if hole_variableA \
+            hole_variable \
+        elseif hole_variable \
+            hole_variable\
+        elseif hole_variableA \
+            hole_variable \
+        end",
+        "Unreachable branch.")
+end

test/rai_rules_tests.jl

@@ -614,6 +614,60 @@ end
     # Splatting
     @test t("f(a...)", "hole_variable(hole_variable_star...)")
     @test t("hcat([f(x) for x in r]...)", "hole_variable([hole_variable(hole_variable_star) for hole_variable in hole_variable]...)")
+
+    # Named variable holes
+    @test t("1 + 2", "1+hole_variableA")
+    @test t("1 + 2", "1+hole_variableB")
+    @test t("1 + 2 + 2", "1+hole_variableA+hole_variableA")
+    @test !t("1 + 2 + 2", "1+hole_variableA+hole_variableB")
+    @test t("1 + 2 + 3", "1+hole_variableA+hole_variableB")
+    @test t("1 + 2 + 3 + 2", "1+hole_variableA+hole_variable +hole_variableA")
+    @test t("1 + 2 + 3 + 2 + 10", "1+hole_variableA+hole_variable +hole_variableA + hole_variable_star")
+
+    @test !t("""
+        if x == 1
+            return 12
+        elseif x== 2
+            return "Reachable_branch"
+        end
+        """, """
+        if hole_variableA == hole_variableB
+            hole_variable
+        elseif hole_variableA == hole_variableB
+            hole_variable
+        end
+        """)
+
+    @test t("""
+        if x == 1
+            return 12
+        elseif x== 1
+            return "Reachable_branch"
+        end
+        """, """
+        if hole_variableA == hole_variableB
+            hole_variable
+        elseif hole_variableA == hole_variableB
+            hole_variable
+        end
+        """)
+
+
+    @test t("""
+        if x == 1
+            println("hello")
+            return 12
+        elseif x== 1
+            println("world")
+            return "Reachable_branch"
+        end
+        """, """
+        if hole_variableA == hole_variableB
+            hole_variable
+        elseif hole_variableA == hole_variableB
+            hole_variable
+        end
+        """)
 end
 
 @testset "unsafe functions" begin
@@ -1229,4 +1283,45 @@ end
                f(1, 2)
                """
     @test lint_test(source, "Line 2, column 1: Possible method call error: f.")
+end
+
+@testset "Branch" begin
+    @testset "Reachable branches" begin
+        source = """
+            function f(x)
+                if x == 1
+                    return 12
+                elseif x== 2
+                    return "Reachable_branch"
+                end
+            end
+            """
+        @test !lint_has_error_test(source)
+    end
+
+    @testset "Unreachable branches" begin
+        source = """
+            function f(x)
+                if x == 1
+                    return 12
+                elseif x== 1
+                    return "Unreachable_branch"
+                end
+            end
+            """
+        @test lint_test(source, "Line 2, column 5: Unreachable branch.")
+    end
+
+    @testset "Unreachable branches 02" begin
+        source = """
+            function f(x)
+                if x <= 1
+                    return 12
+                elseif x <= 1
+                    return "Unreachable_branch"
+                end
+            end
+            """
+        @test lint_test(source, "Line 2, column 5: Unreachable branch.")
+    end
 end