-
Notifications
You must be signed in to change notification settings - Fork 425
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add Warning Check to Evaluate Total Field amount in Structs
- Loading branch information
Showing
2 changed files
with
155 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
defmodule Credo.Check.Warning.StructFieldAmount do | ||
@moduledoc false | ||
|
||
use Credo.Check, | ||
id: "EX????", | ||
base_priority: :normal, | ||
category: :warning, | ||
explanations: [ | ||
check: """ | ||
Structs in Elixir are implemented as compile-time maps, which have a predefined amount of fields. | ||
When structs have 32 or more fields, their internal representation in the Erlang Virtual Machines | ||
changes, potentially leading to bloating and higher memory usage. | ||
""" | ||
] | ||
|
||
alias Credo.Code.Name | ||
@doc false | ||
@impl true | ||
def run(%SourceFile{} = source_file, params) do | ||
issue_meta = IssueMeta.for(source_file, params) | ||
Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta)) | ||
end | ||
|
||
defp traverse( | ||
{:defmodule, _, [{:__aliases__, _, _aliases} | _] = ast}, | ||
issues, | ||
issue_meta | ||
) do | ||
case Macro.prewalk(ast, [], &find_structs_with_32_fields/2) do | ||
{ast, []} -> | ||
{ast, issues} | ||
|
||
{ast, structs} -> | ||
issues = | ||
Enum.reduce(structs, issues, fn {curr, meta}, acc -> | ||
[issue_for(issue_meta, meta, curr) | acc] | ||
end) | ||
|
||
{ast, issues} | ||
end | ||
end | ||
|
||
defp traverse(ast, issues, _issue_meta) do | ||
{ast, issues} | ||
end | ||
|
||
defp find_structs_with_32_fields( | ||
[ | ||
{:__aliases__, meta, aliases}, | ||
[do: {:defstruct, _, [fields]}] | ||
], | ||
acc | ||
) do | ||
if length(fields) >= 32 do | ||
{[], [{Name.full(aliases), meta} | acc]} | ||
else | ||
{[], acc} | ||
end | ||
end | ||
|
||
defp find_structs_with_32_fields(ast, acc) do | ||
{ast, acc} | ||
end | ||
|
||
defp issue_for(issue_meta, meta, struct) do | ||
format_issue(issue_meta, | ||
message: "Struct %#{struct}{} found to have more than 32 fields.", | ||
trigger: "#{struct} do", | ||
line_no: meta[:line], | ||
column: meta[:column] | ||
) | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
defmodule Credo.Check.Warning.StructFieldAmountTest do | ||
use Credo.Test.Case | ||
|
||
@described_check Credo.Check.Warning.StructFieldAmount | ||
|
||
@large_struct_module """ | ||
defmodule MyApp.LargeStruct do | ||
defstruct field_1: "field_1", | ||
field_2: "field_2", | ||
field_3: "field_3", | ||
field_4: "field_4", | ||
field_5: "field_5", | ||
field_6: "field_6", | ||
field_7: "field_7", | ||
field_8: "field_8", | ||
field_9: "field_9", | ||
field_10: "field_10", | ||
field_11: "field_11", | ||
field_12: "field_12", | ||
field_13: "field_13", | ||
field_14: "field_14", | ||
field_15: "field_15", | ||
field_16: "field_16", | ||
field_17: "field_17", | ||
field_18: "field_18", | ||
field_19: "field_19", | ||
field_20: "field_20", | ||
field_21: "field_21", | ||
field_22: "field_22", | ||
field_23: "field_23", | ||
field_24: "field_24", | ||
field_25: "field_25", | ||
field_26: "field_26", | ||
field_27: "field_27", | ||
field_28: "field_28", | ||
field_29: "field_29", | ||
field_30: "field_30", | ||
field_31: "field_31", | ||
field_32: "field_32" | ||
end | ||
""" | ||
|
||
@small_struct_module """ | ||
defmodule MyApp.SmallStruct do | ||
defstruct [field_1: "field_1"] | ||
end | ||
""" | ||
|
||
# | ||
# cases NOT raising issues | ||
# | ||
|
||
test "it should NOT report an issue if the struct has fewer than 32 fields" do | ||
[ | ||
@small_struct_module | ||
] | ||
|> to_source_files() | ||
|> run_check(@described_check) | ||
|> refute_issues() | ||
end | ||
|
||
# | ||
# cases raising issues | ||
# | ||
|
||
test "it should report an issue if a struct has 32 or more fields" do | ||
[ | ||
@large_struct_module | ||
] | ||
|> to_source_files() | ||
|> run_check(@described_check) | ||
|> assert_issue(fn issue -> | ||
assert %{ | ||
line_no: 1, | ||
message: "Struct %MyApp.LargeStruct{} found to have more than 32 fields." | ||
} = issue | ||
|
||
assert issue.trigger == "MyApp.LargeStruct do" | ||
assert issue.column == 11 | ||
end) | ||
end | ||
end |