Skip to content

Commit

Permalink
Interpolation prohibited outside @safe
Browse files Browse the repository at this point in the history
  • Loading branch information
@bergel
bergel committed Sep 24, 2024
1 parent 54719b4 commit 97ed200
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 80 deletions.
13 changes: 13 additions & 0 deletions src/linting/extended_checks.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ struct RelPathAPIUsage_Extension <: ViolationExtendedRule end
struct NonFrontShapeAPIUsage_Extension <: ViolationExtendedRule end
struct InterpolationInSafeLog_Extension <: RecommendationExtendedRule end
struct UseOfStaticThreads <: ViolationExtendedRule end
struct InterpolationOnlyInSafe <: ViolationExtendedRule end


const all_extended_rule_types = Ref{Any}(
Expand Down Expand Up @@ -576,3 +577,15 @@ function check(t::UseOfStaticThreads, x::EXPR)
generic_check(t, x, "@threads :static hole_variable_star", msg)
generic_check(t, x, "Threads.@threads :static hole_variable_star", msg)
end

function check(t::InterpolationOnlyInSafe, x::EXPR, markers::Dict{Symbol,String})
# We are in a macro call and the macro is @safe, we merely exit
haskey(markers, :macrocall) && markers[:macrocall] == "@safe" && return

# We are not in a @safe macro call, so we need to check for interpolation
msg = raw"""
Log messages must always be constructed via @safe("..") strings. If this interpolation is used in a log message, it should be a @safe-string. Please try this instead: @safe("...$(x)..."). If this is not being used for logging, you can lint-ignore this line.
"""

generic_check(t, x, "\"LINT_STRING_WITH_INTERPOLATION\"", msg)
end
176 changes: 96 additions & 80 deletions test/rai_rules_tests.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,7 @@ end
# LINT_STRING_WITH_INTERPOLATION
@test !t("\"1 + 2\"", "\"LINT_STRING_WITH_INTERPOLATION\"")
@test t(raw"\"foo $x\"", "\"LINT_STRING_WITH_INTERPOLATION\"")
@test t(raw"\"foo $(x)\"", "\"LINT_STRING_WITH_INTERPOLATION\"")
@test t(raw"@warnv_safe_to_log 1 \"logged! $(a_variable)\"", "@warnv_safe_to_log hole_variable \"LINT_STRING_WITH_INTERPOLATION\"")
@test !t(raw"@warnv_safe_to_log 1 \"logged! (a_variable)\"", "@warnv_safe_to_log hole_variable \"LINT_STRING_WITH_INTERPOLATION\"")

Expand Down Expand Up @@ -1783,86 +1784,86 @@ end
@test !isnothing(match(expected, result))
end

@testset "Checking string interpolation" begin

# ERRORS
source_with_error = raw"""
function f(conf)
@info "($conf.container.baseurl)"
end
"""

source_with_error2 = raw"""
function f(conf)
@info "$conf.container.baseurl"
end
"""

source_with_error3 = raw"""
function f(conf)
@info "this string contains an error $conf.container.baseurl indeed!"
end
"""

source_with_error4 = raw"""
function f(conf)
@info "this string contains an error $conf .container.baseurl indeed!"
end
"""

source_with_error5 = raw"""
function f(engine_name)
@info "Issuing delete request for engine $engine_name..."
end
"""

source_with_error6 = raw"""
function f()
Source("model/$name", "model/$name", read(joinpath(@__DIR__, "models", "$name.rel"), String))
end
"""

source_with_error7 = raw"""
function f()
path = "$dir/$name.csv"
end
"""

@test lint_test(source_with_error, raw"Line 2, column 11: Use $(x) instead of $x ")
@test lint_test(source_with_error2, raw"Line 2, column 11: Use $(x) instead of $x ")
@test lint_test(source_with_error3, raw"Line 2, column 11: Use $(x) instead of $x ")
@test lint_test(source_with_error4, raw"Line 2, column 11: Use $(x) instead of $x ")
@test lint_test(source_with_error5, raw"Line 2, column 11: Use $(x) instead of $x ")

@test lint_test(source_with_error6, raw"Line 2, column 12: Use $(x) instead of $x ")
@test lint_test(source_with_error6, raw"Line 2, column 27: Use $(x) instead of $x ")
@test lint_test(source_with_error6, raw"Line 2, column 77: Use $(x) instead of $x ")

@test lint_test(source_with_error7, raw"Line 2, column 12: Use $(x) instead of $x ")

# NO ERROR
source_without_error = raw"""
function f(conf)
@info "$(conf.container.baseurl)"
end
"""

source_without_error2 = raw"""
function f(conf)
@info "this string contains an error $(conf.container.baseurl) indeed!"
end
"""

source_without_error3 = raw"""
function f()
_profile_filename = "profile-$(timestamp).pb.gz"
end
"""

@test count_lint_errors(source_without_error) == 0
@test count_lint_errors(source_without_error2) == 0
@test count_lint_errors(source_without_error3) == 0
end
# @testset "Checking string interpolation" begin

# # ERRORS
# source_with_error = raw"""
# function f(conf)
# @info "($conf.container.baseurl)"
# end
# """

# source_with_error2 = raw"""
# function f(conf)
# @info "$conf.container.baseurl"
# end
# """

# source_with_error3 = raw"""
# function f(conf)
# @info "this string contains an error $conf.container.baseurl indeed!"
# end
# """

# source_with_error4 = raw"""
# function f(conf)
# @info "this string contains an error $conf .container.baseurl indeed!"
# end
# """

# source_with_error5 = raw"""
# function f(engine_name)
# @info "Issuing delete request for engine $engine_name..."
# end
# """

# source_with_error6 = raw"""
# function f()
# Source("model/$name", "model/$name", read(joinpath(@__DIR__, "models", "$name.rel"), String))
# end
# """

# source_with_error7 = raw"""
# function f()
# path = "$dir/$name.csv"
# end
# """

# @test lint_test(source_with_error, raw"Line 2, column 11: Use $(x) instead of $x ")
# @test lint_test(source_with_error2, raw"Line 2, column 11: Use $(x) instead of $x ")
# @test lint_test(source_with_error3, raw"Line 2, column 11: Use $(x) instead of $x ")
# @test lint_test(source_with_error4, raw"Line 2, column 11: Use $(x) instead of $x ")
# @test lint_test(source_with_error5, raw"Line 2, column 11: Use $(x) instead of $x ")

# @test lint_test(source_with_error6, raw"Line 2, column 12: Use $(x) instead of $x ")
# @test lint_test(source_with_error6, raw"Line 2, column 27: Use $(x) instead of $x ")
# @test lint_test(source_with_error6, raw"Line 2, column 77: Use $(x) instead of $x ")

# @test lint_test(source_with_error7, raw"Line 2, column 12: Use $(x) instead of $x ")

# # NO ERROR
# source_without_error = raw"""
# function f(conf)
# @info "$(conf.container.baseurl)"
# end
# """

# source_without_error2 = raw"""
# function f(conf)
# @info "this string contains an error $(conf.container.baseurl) indeed!"
# end
# """

# source_without_error3 = raw"""
# function f()
# _profile_filename = "profile-$(timestamp).pb.gz"
# end
# """

# @test count_lint_errors(source_without_error) == 0
# @test count_lint_errors(source_without_error2) == 0
# @test count_lint_errors(source_without_error3) == 0
# end

@testset "Arithmetic LintResult" begin
l1 = LintResult()
Expand Down Expand Up @@ -1988,3 +1989,18 @@ end
@test lint_test(source, "Line 2, column 5: Use `Threads.@threads :dynamic` instead of `Threads.@threads :static`.")
@test lint_test(source, "Line 6, column 5: Use `Threads.@threads :dynamic` instead of `Threads.@threads :static`.")
end


@testset "Interpolation prohibited outside @safe" begin
source = raw"""
function f()
print("...$(x)...")
@info @safe("...$(x)...")
@safe("...$(y)...")
end
"""
@test lint_test(source, raw"""Line 2, column 11: Log messages must always be constructed via @safe("..") strings. If this interpolation is used in a log message, it should be a @safe-string. Please try this instead: @safe("...$(x)..."). If this is not being used for logging, you can lint-ignore this line.""")

# There is only one lint error in source, which is in Line 2 as tested just above
@test count_lint_errors(source) == 1
end

0 comments on commit 97ed200

Please sign in to comment.