From aed905daf000c29dfab4a540c805e9eaf2f0c415 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Sat, 23 Nov 2024 01:45:51 -0800 Subject: [PATCH 1/5] Add support for linting ECR files Requires Crystal >= 1.15.0 --- README.md | 1 + src/ameba/config.cr | 19 ++++++++++++++----- src/ameba/glob_utils.cr | 5 +++++ src/ameba/source.cr | 18 ++++++++++++++++++ 4 files changed, 38 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 58b74e6e0..729ecfca0 100644 --- a/README.md +++ b/README.md @@ -174,6 +174,7 @@ In this example we define default globs and exclude `src/compiler` folder: ``` yaml Globs: - "**/*.cr" + - "**/*.ecr" - "!lib" Excluded: diff --git a/src/ameba/config.cr b/src/ameba/config.cr index 89a710e53..c1832f702 100644 --- a/src/ameba/config.cr +++ b/src/ameba/config.cr @@ -1,5 +1,6 @@ require "semantic_version" require "yaml" +require "ecr/processor" require "./glob_utils" # A configuration entry for `Ameba::Runner`. @@ -57,10 +58,18 @@ class Ameba::Config Path[XDG_CONFIG_HOME] / "ameba/config.yml", } - DEFAULT_GLOBS = %w( - **/*.cr - !lib - ) + {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %} + DEFAULT_GLOBS = %w( + **/*.cr + **/*.ecr + !lib + ) + {% else %} + DEFAULT_GLOBS = %w( + **/*.cr + !lib + ) + {% end %} getter rules : Array(Rule::Base) property severity = Severity::Convention @@ -167,7 +176,7 @@ class Ameba::Config # ``` # config = Ameba::Config.load # config.sources # => list of default sources - # config.globs = ["**/*.cr"] + # config.globs = ["**/*.cr", "**/*.ecr"] # config.excluded = ["spec"] # config.sources # => list of sources pointing to files found by the wildcards # ``` diff --git a/src/ameba/glob_utils.cr b/src/ameba/glob_utils.cr index 63dd67cc7..60d41bd72 100644 --- a/src/ameba/glob_utils.cr +++ b/src/ameba/glob_utils.cr @@ -25,6 +25,11 @@ module Ameba globs .flat_map do |glob| glob += "/**/*.cr" if File.directory?(glob) + + {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %} + glob += "/**/*.ecr" if File.directory?(glob) + {% end %} + Dir[glob] end .uniq! diff --git a/src/ameba/source.cr b/src/ameba/source.cr index 29d9d1990..48f573d08 100644 --- a/src/ameba/source.cr +++ b/src/ameba/source.cr @@ -57,6 +57,24 @@ module Ameba # source.ast # ``` getter ast : Crystal::ASTNode do + code = @code + + {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %} + if @path.ends_with?(".ecr") + begin + code = ECR.process_string(code, @path) + rescue ex : Crystal::SyntaxException + # Need to rescue to add the filename + raise Crystal::SyntaxException.new( + ex.message, + ex.line_number, + ex.column_number, + @path + ) + end + end + {% end %} + Crystal::Parser.new(code) .tap(&.wants_doc = true) .tap(&.filename = path) From b60a03fa10ae1c72416fdaf29b4d67236f6c1524 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Sun, 12 Jan 2025 09:51:06 -0500 Subject: [PATCH 2/5] Apply suggestions from code review Co-authored-by: Sijawusz Pur Rahnama --- src/ameba/glob_utils.cr | 6 ++++-- src/ameba/source.cr | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/ameba/glob_utils.cr b/src/ameba/glob_utils.cr index 60d41bd72..393663d3c 100644 --- a/src/ameba/glob_utils.cr +++ b/src/ameba/glob_utils.cr @@ -24,11 +24,13 @@ module Ameba def expand(globs) globs .flat_map do |glob| - glob += "/**/*.cr" if File.directory?(glob) + if File.directory?(glob) + glob += "/**/*.cr" {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %} - glob += "/**/*.ecr" if File.directory?(glob) + glob += "/**/*.ecr" {% end %} + end Dir[glob] end diff --git a/src/ameba/source.cr b/src/ameba/source.cr index 48f573d08..5f737a601 100644 --- a/src/ameba/source.cr +++ b/src/ameba/source.cr @@ -63,7 +63,7 @@ module Ameba if @path.ends_with?(".ecr") begin code = ECR.process_string(code, @path) - rescue ex : Crystal::SyntaxException + rescue ex : ECR::Lexer::SyntaxException # Need to rescue to add the filename raise Crystal::SyntaxException.new( ex.message, From c0967fd8d93c973c9f39c48a8b3fe6b6e81967cd Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Sun, 12 Jan 2025 15:30:25 -0500 Subject: [PATCH 3/5] Add some specs related to ECR files --- .../rule/layout/trailing_blank_lines_spec.cr | 7 +++++ .../rule/layout/trailing_whitespace_spec.cr | 11 ++++++++ spec/ameba/rule/lint/formatting_spec.cr | 13 ++++++++++ .../rule/lint/literal_in_condition_spec.cr | 12 +++++++++ spec/ameba/rule/lint/unused_literal_spec.cr | 15 +++++++++++ spec/ameba/source_spec.cr | 26 +++++++++++++++++++ 6 files changed, 84 insertions(+) diff --git a/spec/ameba/rule/layout/trailing_blank_lines_spec.cr b/spec/ameba/rule/layout/trailing_blank_lines_spec.cr index 028ab496b..05e53845e 100644 --- a/spec/ameba/rule/layout/trailing_blank_lines_spec.cr +++ b/spec/ameba/rule/layout/trailing_blank_lines_spec.cr @@ -52,5 +52,12 @@ module Ameba::Rule::Layout issue.message.should eq "Trailing newline missing" end end + + {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %} + it "fails if there more then one blank line at the end of an ECR source" do + source = expect_issue subject, "a = 1\n \n # error: Excessive trailing newline detected", "source.ecr" + expect_no_corrections source + end + {% end %} end end diff --git a/spec/ameba/rule/layout/trailing_whitespace_spec.cr b/spec/ameba/rule/layout/trailing_whitespace_spec.cr index df376870f..4c326c91e 100644 --- a/spec/ameba/rule/layout/trailing_whitespace_spec.cr +++ b/spec/ameba/rule/layout/trailing_whitespace_spec.cr @@ -15,5 +15,16 @@ module Ameba::Rule::Layout expect_correction source, "whitespace at the end" end + + {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %} + it "fails if there is a line with trailing whitespace in an ECR file" do + source = expect_issue subject, + "whitespace at the end \n" \ + " # ^^ error: Trailing whitespace detected", + path: "source.ecr" + + expect_correction source, "whitespace at the end" + end + {% end %} end end diff --git a/spec/ameba/rule/lint/formatting_spec.cr b/spec/ameba/rule/lint/formatting_spec.cr index 054f3c1f3..cdb667fe1 100644 --- a/spec/ameba/rule/lint/formatting_spec.cr +++ b/spec/ameba/rule/lint/formatting_spec.cr @@ -53,5 +53,18 @@ module Ameba::Rule::Lint end end end + + {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %} + it "passes ECR files regardless of formatting" do + source = expect_no_issues subject, <<-CRYSTAL, "source.ecr" + <% + def method(a,b,c=0) + a+b+c + end + %> + + CRYSTAL + end + {% end %} end end diff --git a/spec/ameba/rule/lint/literal_in_condition_spec.cr b/spec/ameba/rule/lint/literal_in_condition_spec.cr index 8150ad2bd..307f0a389 100644 --- a/spec/ameba/rule/lint/literal_in_condition_spec.cr +++ b/spec/ameba/rule/lint/literal_in_condition_spec.cr @@ -175,5 +175,17 @@ module Ameba::Rule::Lint issue.end_location.to_s.should eq "source.cr:1:20" issue.message.should eq "Literal value found in conditional" end + + {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %} + it "fails if there is a predicate in if conditional in an ECR file" do + s = Source.new %( + <% if "string" %> + :ok + <% end %> + ), "source.ecr" + + subject.catch(s).should_not be_valid + end + {% end %} end end diff --git a/spec/ameba/rule/lint/unused_literal_spec.cr b/spec/ameba/rule/lint/unused_literal_spec.cr index 5bf63f56f..8cd74dd1a 100644 --- a/spec/ameba/rule/lint/unused_literal_spec.cr +++ b/spec/ameba/rule/lint/unused_literal_spec.cr @@ -295,5 +295,20 @@ module Ameba::Rule::Lint CRYSTAL end {% end %} + + {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %} + it "passes if a string is used in an assign directive in an ECR file" do + expect_no_issues subject, <<-CRYSTAL, path: "source.ecr" + <%= "hello world" %> + CRYSTAL + end + + it "fails if a string is unused in an ECR file" do + expect_issue subject, <<-CRYSTAL, path: "source.ecr" + <% "hello world" %> + # ^^^^^^^^^^^^^ error: Literal value is not used + CRYSTAL + end + {% end %} end end diff --git a/spec/ameba/source_spec.cr b/spec/ameba/source_spec.cr index 8c8b1a1b4..08f8f2e78 100644 --- a/spec/ameba/source_spec.cr +++ b/spec/ameba/source_spec.cr @@ -70,5 +70,31 @@ module Ameba CRYSTAL end end + + {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %} + describe "#ast" do + it "parses an ECR file" do + source = Source.new <<-ECR, "filename.ecr" + hello <%= "world" %> + ECR + + source.ast.to_s.should eq(<<-CRYSTAL) + __str__ << "hello " + ("world").to_s(__str__) + + CRYSTAL + end + + it "raises an exception when ECR parsing fails" do + source = Source.new <<-ECR, "filename.ecr" + hello <%= "world" > + ECR + + expect_raises(Crystal::SyntaxException) do + source.ast + end + end + end + {% end %} end end From ecab603d2970e496a0070adc196712bba638afb9 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Wed, 15 Jan 2025 10:38:46 -0500 Subject: [PATCH 4/5] Remove unnecessary ECR specs --- .../rule/layout/trailing_blank_lines_spec.cr | 7 ------- .../ameba/rule/layout/trailing_whitespace_spec.cr | 11 ----------- spec/ameba/rule/lint/formatting_spec.cr | 13 ------------- spec/ameba/rule/lint/literal_in_condition_spec.cr | 12 ------------ spec/ameba/rule/lint/unused_literal_spec.cr | 15 --------------- 5 files changed, 58 deletions(-) diff --git a/spec/ameba/rule/layout/trailing_blank_lines_spec.cr b/spec/ameba/rule/layout/trailing_blank_lines_spec.cr index 05e53845e..028ab496b 100644 --- a/spec/ameba/rule/layout/trailing_blank_lines_spec.cr +++ b/spec/ameba/rule/layout/trailing_blank_lines_spec.cr @@ -52,12 +52,5 @@ module Ameba::Rule::Layout issue.message.should eq "Trailing newline missing" end end - - {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %} - it "fails if there more then one blank line at the end of an ECR source" do - source = expect_issue subject, "a = 1\n \n # error: Excessive trailing newline detected", "source.ecr" - expect_no_corrections source - end - {% end %} end end diff --git a/spec/ameba/rule/layout/trailing_whitespace_spec.cr b/spec/ameba/rule/layout/trailing_whitespace_spec.cr index 4c326c91e..df376870f 100644 --- a/spec/ameba/rule/layout/trailing_whitespace_spec.cr +++ b/spec/ameba/rule/layout/trailing_whitespace_spec.cr @@ -15,16 +15,5 @@ module Ameba::Rule::Layout expect_correction source, "whitespace at the end" end - - {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %} - it "fails if there is a line with trailing whitespace in an ECR file" do - source = expect_issue subject, - "whitespace at the end \n" \ - " # ^^ error: Trailing whitespace detected", - path: "source.ecr" - - expect_correction source, "whitespace at the end" - end - {% end %} end end diff --git a/spec/ameba/rule/lint/formatting_spec.cr b/spec/ameba/rule/lint/formatting_spec.cr index cdb667fe1..054f3c1f3 100644 --- a/spec/ameba/rule/lint/formatting_spec.cr +++ b/spec/ameba/rule/lint/formatting_spec.cr @@ -53,18 +53,5 @@ module Ameba::Rule::Lint end end end - - {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %} - it "passes ECR files regardless of formatting" do - source = expect_no_issues subject, <<-CRYSTAL, "source.ecr" - <% - def method(a,b,c=0) - a+b+c - end - %> - - CRYSTAL - end - {% end %} end end diff --git a/spec/ameba/rule/lint/literal_in_condition_spec.cr b/spec/ameba/rule/lint/literal_in_condition_spec.cr index 307f0a389..8150ad2bd 100644 --- a/spec/ameba/rule/lint/literal_in_condition_spec.cr +++ b/spec/ameba/rule/lint/literal_in_condition_spec.cr @@ -175,17 +175,5 @@ module Ameba::Rule::Lint issue.end_location.to_s.should eq "source.cr:1:20" issue.message.should eq "Literal value found in conditional" end - - {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %} - it "fails if there is a predicate in if conditional in an ECR file" do - s = Source.new %( - <% if "string" %> - :ok - <% end %> - ), "source.ecr" - - subject.catch(s).should_not be_valid - end - {% end %} end end diff --git a/spec/ameba/rule/lint/unused_literal_spec.cr b/spec/ameba/rule/lint/unused_literal_spec.cr index 8cd74dd1a..5bf63f56f 100644 --- a/spec/ameba/rule/lint/unused_literal_spec.cr +++ b/spec/ameba/rule/lint/unused_literal_spec.cr @@ -295,20 +295,5 @@ module Ameba::Rule::Lint CRYSTAL end {% end %} - - {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %} - it "passes if a string is used in an assign directive in an ECR file" do - expect_no_issues subject, <<-CRYSTAL, path: "source.ecr" - <%= "hello world" %> - CRYSTAL - end - - it "fails if a string is unused in an ECR file" do - expect_issue subject, <<-CRYSTAL, path: "source.ecr" - <% "hello world" %> - # ^^^^^^^^^^^^^ error: Literal value is not used - CRYSTAL - end - {% end %} end end From 7614001f133b36a1980532bf6752d1928bae93e9 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Wed, 15 Jan 2025 11:47:04 -0500 Subject: [PATCH 5/5] Macro helper method for ECR support --- spec/ameba/source_spec.cr | 4 ++-- src/ameba.cr | 22 ++++++++++++++-------- src/ameba/config.cr | 20 ++++++++------------ src/ameba/glob_utils.cr | 12 ++++++------ src/ameba/source.cr | 4 ++-- 5 files changed, 32 insertions(+), 30 deletions(-) diff --git a/spec/ameba/source_spec.cr b/spec/ameba/source_spec.cr index 08f8f2e78..b653a8c8d 100644 --- a/spec/ameba/source_spec.cr +++ b/spec/ameba/source_spec.cr @@ -71,7 +71,7 @@ module Ameba end end - {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %} + Ameba.ecr_supported? do describe "#ast" do it "parses an ECR file" do source = Source.new <<-ECR, "filename.ecr" @@ -95,6 +95,6 @@ module Ameba end end end - {% end %} + end end end diff --git a/src/ameba.cr b/src/ameba.cr index e675939ac..9f4d689e9 100644 --- a/src/ameba.cr +++ b/src/ameba.cr @@ -1,11 +1,3 @@ -require "./ameba/*" -require "./ameba/ast/**" -require "./ameba/ext/**" -require "./ameba/rule/**" -require "./ameba/formatter/*" -require "./ameba/presenter/*" -require "./ameba/source/**" - # Ameba's entry module. # # To run the linter with default parameters: @@ -40,4 +32,18 @@ module Ameba def run(config = Config.load) Runner.new(config).run end + + macro ecr_supported?(&) + {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %} + {{ yield }} + {% end %} + end end + +require "./ameba/*" +require "./ameba/ast/**" +require "./ameba/ext/**" +require "./ameba/rule/**" +require "./ameba/formatter/*" +require "./ameba/presenter/*" +require "./ameba/source/**" diff --git a/src/ameba/config.cr b/src/ameba/config.cr index c1832f702..d09fff347 100644 --- a/src/ameba/config.cr +++ b/src/ameba/config.cr @@ -58,18 +58,14 @@ class Ameba::Config Path[XDG_CONFIG_HOME] / "ameba/config.yml", } - {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %} - DEFAULT_GLOBS = %w( - **/*.cr - **/*.ecr - !lib - ) - {% else %} - DEFAULT_GLOBS = %w( - **/*.cr - !lib - ) - {% end %} + DEFAULT_GLOBS = %w( + **/*.cr + !lib + ) + + Ameba.ecr_supported? do + DEFAULT_GLOBS << "**/*.ecr" + end getter rules : Array(Rule::Base) property severity = Severity::Convention diff --git a/src/ameba/glob_utils.cr b/src/ameba/glob_utils.cr index 393663d3c..cee311b46 100644 --- a/src/ameba/glob_utils.cr +++ b/src/ameba/glob_utils.cr @@ -24,13 +24,13 @@ module Ameba def expand(globs) globs .flat_map do |glob| - if File.directory?(glob) - glob += "/**/*.cr" + if File.directory?(glob) + glob += "/**/*.cr" - {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %} - glob += "/**/*.ecr" - {% end %} - end + Ameba.ecr_supported? do + glob += "/**/*.ecr" + end + end Dir[glob] end diff --git a/src/ameba/source.cr b/src/ameba/source.cr index 5f737a601..89255de0c 100644 --- a/src/ameba/source.cr +++ b/src/ameba/source.cr @@ -59,7 +59,7 @@ module Ameba getter ast : Crystal::ASTNode do code = @code - {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %} + Ameba.ecr_supported? do if @path.ends_with?(".ecr") begin code = ECR.process_string(code, @path) @@ -73,7 +73,7 @@ module Ameba ) end end - {% end %} + end Crystal::Parser.new(code) .tap(&.wants_doc = true)