Skip to content

Check on return type #63

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.

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

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion src/interface.jl
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
end

function is_already_linted(l::LintResult, filename)
return filename in l.linted_files

Check failure on line 51 in src/interface.jl

View workflow job for this annotation

GitHub Actions / run_lint

Use `tin(item,collection)` instead of the Julia's `in` or `∈`. raicode/src/interface.jl
end

function has_values(l::LintResult, a, b, c)
Expand Down Expand Up @@ -196,7 +196,7 @@
this_annotation = annotation_for_this_line(line)
# current_codepoint + sizeof(line) is possibly pointing at the newline that isn't
# actually stored in `line`.
if offset in current_codepoint:(current_codepoint + sizeof(line))

Check failure on line 199 in src/interface.jl

View workflow job for this annotation

GitHub Actions / run_lint

Use `tin(item,collection)` instead of the Julia's `in` or `∈`. raicode/src/interface.jl
index_in_line = offset - current_codepoint + 1 # possibly off the end by 1.
if !isnothing(this_annotation)
annotation = this_annotation
Expand Down Expand Up @@ -276,7 +276,7 @@
# Filter along the file content
ss = split(hint_as_string)
has_filename = isfile(last(ss))
has_filename || error("Should have a filename")

Check failure on line 279 in src/interface.jl

View workflow job for this annotation

GitHub Actions / run_lint

Use custom exception instead of the generic `error()`. raicode/src/interface.jl

filename = string(last(ss))

Expand All @@ -295,7 +295,6 @@
if has_no_annotation
# No annotation, so we merely print the reported error.
if should_print_hint(lint_result)
# isdefined(Main, :Infiltrator) && Main.infiltrate(@__MODULE__, Base.@locals, @__FILE__, @__LINE__)
print_hint(formatter, io, "Line $(line_number), column $(column):", cleaned_hint)
lint_result.printout_count += 1
end
Expand Down Expand Up @@ -463,7 +462,7 @@
end

# If already linted, then we merely exit
rootpath in result.linted_files && return result

Check failure on line 465 in src/interface.jl

View workflow job for this annotation

GitHub Actions / run_lint

Use `tin(item,collection)` instead of the Julia's `in` or `∈`. raicode/src/interface.jl

# If we are running Lint on a directory
isdir(rootpath) && return _run_lint_on_dir(rootpath; result, server, io, io_violations, io_recommendations, filters, formatter)
Expand Down
11 changes: 11 additions & 0 deletions src/linting/extended_checks.jl
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
!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"))

Check failure on line 58 in src/linting/extended_checks.jl

View workflow job for this annotation

GitHub Actions / run_lint

Use $(x) instead of $x ([explanation](https://github.com/RelationalAI/RAIStyle?tab=readme-ov-file#string-interpolation)). raicode/src/linting/extended_checks.jl
if contains(x, "QQQ")
reg_exp = Regex(replace(x, "QQQ" => ".*"))
return !isnothing(match(reg_exp, y))
Expand Down Expand Up @@ -209,6 +209,7 @@
struct UnreachableBranch_Extension <: ViolationExtendedRule end
struct StringInterpolation_Extension <: ViolationExtendedRule end
struct RelPathAPIUsage_Extension <: ViolationExtendedRule end
struct ReturnType_Extension <: ViolationExtendedRule end
struct NonFrontShapeAPIUsage_Extension <: ViolationExtendedRule end
struct InterpolationInSafeLog_Extension <: RecommendationExtendedRule end
struct UseOfStaticThreads <: ViolationExtendedRule end
Expand Down Expand Up @@ -268,7 +269,7 @@

function get_recommendation(msg_prefix)
m = retrieve_full_msg_from_prefix(msg_prefix)
m in keys(is_recommendation) || return nothing

Check failure on line 272 in src/linting/extended_checks.jl

View workflow job for this annotation

GitHub Actions / run_lint

Use `tin(item,collection)` instead of the Julia's `in` or `∈`. raicode/src/linting/extended_checks.jl
return is_recommendation[m]
end

Expand Down Expand Up @@ -311,7 +312,7 @@
end

function check_for_recommendation(T::DataType, msg::String)
@assert supertype(T) in [RecommendationExtendedRule, ViolationExtendedRule]

Check failure on line 315 in src/linting/extended_checks.jl

View workflow job for this annotation

GitHub Actions / run_lint

Use `tin(item,collection)` instead of the Julia's `in` or `∈`. raicode/src/linting/extended_checks.jl
b = supertype(T) == RecommendationExtendedRule
get!(is_recommendation, msg, b)
return nothing
Expand Down Expand Up @@ -343,7 +344,7 @@

function check(t::NThreads_Extention, x::EXPR, markers::Dict{Symbol,String})
# Threads.nthreads() must not be used in a const field, but it is allowed elsewhere
haskey(markers, :const) || return

Check failure on line 347 in src/linting/extended_checks.jl

View workflow job for this annotation

GitHub Actions / run_lint

Use `thaskey(dict,key)` instead of the Julia's `haskey`. raicode/src/linting/extended_checks.jl
generic_check(t, x, "Threads.nthreads()", "`Threads.nthreads()` should not be used in a constant variable.")
end

Expand Down Expand Up @@ -397,10 +398,10 @@
check(t::Ptr_Extension, x::EXPR) = generic_check(t, x, "Ptr{hole_variable}(hole_variable)")

function check(t::ArrayWithNoType_Extension, x::EXPR, markers::Dict{Symbol,String})
haskey(markers, :filename) || return

Check failure on line 401 in src/linting/extended_checks.jl

View workflow job for this annotation

GitHub Actions / run_lint

Use `thaskey(dict,key)` instead of the Julia's `haskey`. raicode/src/linting/extended_checks.jl
contains(markers[:filename], "src/Compiler") || return

haskey(markers, :macrocall) && markers[:macrocall] == "@match" && return

Check failure on line 404 in src/linting/extended_checks.jl

View workflow job for this annotation

GitHub Actions / run_lint

Use `thaskey(dict,key)` instead of the Julia's `haskey`. raicode/src/linting/extended_checks.jl
haskey(markers, :macrocall) && markers[:macrocall] == "@matchrule" && return

generic_check(t, x, "[]", "Need a specific Array type to be provided.")
Expand Down Expand Up @@ -549,6 +550,16 @@
generic_check(t, x, "relpath_from_signature(hole_variable)", "Usage of method `relpath_from_signature` is not allowed in this context.")
end

function check(t::ReturnType_Extension, x::EXPR, markers::Dict{Symbol,String})
# No check of return type in Arroyo
haskey(markers, :filename) || return
contains(markers[:filename], "packages/Arroyo/src/") && return

msg = "Return type are prohibited ([explanation](https://github.com/RelationalAI/RAIStyle?tab=readme-ov-file#return-type-annotations-in-function-signatures-can-cause-runtime-cost))."
generic_check(t, x, "hole_variable(hole_variable_star)::hole_variable = hole_variable", msg)
generic_check(t, x, "function hole_variable(hole_variable_star)::hole_variable hole_variable_star end", msg)
end

function check(t::NonFrontShapeAPIUsage_Extension, x::EXPR, markers::Dict{Symbol,String})
haskey(markers, :filename) || return
# In the front-end and in FFI, we are allowed to refer to `Shape`
Expand Down
40 changes: 40 additions & 0 deletions test/rai_rules_tests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1969,6 +1969,46 @@ end
@test lint_test(source, "Line 10, column 17: Safe warning log has interpolation.")
end

@testset "Prohibit return types" begin
@testset "Case 01" begin
source = raw"""
function pm_check_mutable_pages(bytes::Int)::Bool
pm = PAGER_MONITOR

max_pages = (@atomic pm.mutable_pages_running_max)
max_bytes = (@atomic pm.mutable_bytes_running_max)
if max_bytes >= bytes
@warnv_safe_to_log 0 "[Pager] Too many mutable pages \
detected: $max_pages pages weighting $max_bytes bytes"

@ensure @warnv_safe_to_log 2 "[Pager] Too many mutable pages \
detected: $max_pages pages weighting $max_bytes bytes"

@ensure @warnv_safe_to_log 0 "no interpolation"

mutable_report = pm_generate_mutable_pages_report!()
if !isnothing(mutable_report)
@warnv_safe_to_log 0 mutable_report
end
return false
end

return true
end
"""
@test lint_test(source, "Line 1, column 1: Return type are prohibited")
end

@testset "Case 02" begin
source = """
function Base.foo()::Int
return 42
end
"""
@test lint_test(source, "Line 1, column 1: Return type are prohibited")
end
end

@testset "Use of static threads" begin
source = raw"""
function f()
Expand Down
Loading