Skip to content
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

Refactors before 1.7 release #555

Merged
merged 29 commits into from
Jan 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
da33d70
`eq true` -> `be_true`
Sija Dec 29, 2024
6af9e1c
Cleanup redundant `:nodoc:` annotations
Sija Dec 29, 2024
6a70b09
Fix code indentation in specs
Sija Dec 29, 2024
ca5b806
Correctly wrap typo suggestions in backticks
Sija Dec 29, 2024
360f7d7
Change `Documentation` rule group severity to `Convention` instead of…
Sija Dec 29, 2024
7c38b88
Change `Documentation/DocumentationAdmonition` rule severity to `Warn…
Sija Dec 29, 2024
954493f
Add missing `describe` blocks
Sija Dec 29, 2024
68c3852
Keep `subject` assignments within the `describe` block
Sija Dec 29, 2024
565988e
Refactor formatters’ specs
Sija Dec 29, 2024
34966ed
Make some internal methods private
Sija Jan 8, 2025
b3884e9
Remove `prefer_name_location: true` since it doesn’t change anything
Sija Jan 12, 2025
0be073f
Remove some `ameba:disable Lint/NotNil` directives
Sija Jan 12, 2025
fb87aa3
Return early in `Lint/Formatting` if source code is empty
Sija Jan 12, 2025
78c0f76
Cleanup `AST::Reference` spec
Sija Jan 12, 2025
9d0d564
Add `Rule::Lint::Typos#fail_on_missing_bin?` option
Sija Jan 12, 2025
69f9f8c
`eq nil` -> `be_nil`
Sija Jan 12, 2025
eaeebd9
Move `ecr_supported?` macro to the top of the module
Sija Jan 15, 2025
fb80d37
Add `Source#ecr?` method
Sija Jan 16, 2025
71cc07f
Use `path` getter instead
Sija Jan 16, 2025
2f07b29
Make some methods private
Sija Jan 28, 2025
fe8fba5
Add additional spec assertion
Sija Jan 29, 2025
170fbf6
Wrap long lines for better readability
Sija Jan 29, 2025
131c35e
Refactor visitors a bit
Sija Jan 29, 2025
6ef291d
Use `Enabled: true` in the rules’ YAML configuration example sections
Sija Jan 29, 2025
a961126
Several readability related refactors
Sija Jan 29, 2025
2e8c287
Add backticks around Crystal keywords in some more comments and messages
Sija Jan 29, 2025
074d86e
Move `describe` block outside of `Ameba.ecr_supported?` call
Sija Jan 29, 2025
6b19c0f
Tweak rules’ docs and messages
Sija Jan 29, 2025
cd144e9
Use variant of heredoc without interpolation wherever possible
Sija Jan 30, 2025
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
4 changes: 2 additions & 2 deletions spec/ameba/ast/flow_expression_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ module Ameba::AST
CRYSTAL
node = nodes.expressions_nodes.first
flow_expression = FlowExpression.new node, false
flow_expression.unreachable_nodes.empty?.should eq true
flow_expression.unreachable_nodes.empty?.should be_true
end

it "returns nil if there is no unreachable node" do
Expand All @@ -64,7 +64,7 @@ module Ameba::AST
CRYSTAL
node = nodes.expressions_nodes.first
flow_expression = FlowExpression.new node, false
flow_expression.unreachable_nodes.empty?.should eq true
flow_expression.unreachable_nodes.empty?.should be_true
end
end
end
Expand Down
46 changes: 23 additions & 23 deletions spec/ameba/ast/util_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ module Ameba::AST
include Util
end

subject = Test.new

describe Util do
subject = Test.new

describe "#literal?" do
[
Crystal::ArrayLiteral.new,
Expand Down Expand Up @@ -120,12 +120,12 @@ module Ameba::AST
describe "#flow_command?" do
it "returns true if this is return" do
node = as_node("return 22")
subject.flow_command?(node, false).should eq true
subject.flow_command?(node, false).should be_true
end

it "returns true if this is a break in a loop" do
node = as_node("break")
subject.flow_command?(node, true).should eq true
subject.flow_command?(node, true).should be_true
end

it "returns false if this is a break out of loop" do
Expand All @@ -135,7 +135,7 @@ module Ameba::AST

it "returns true if this is a next in a loop" do
node = as_node("next")
subject.flow_command?(node, true).should eq true
subject.flow_command?(node, true).should be_true
end

it "returns false if this is a next out of loop" do
Expand All @@ -145,17 +145,17 @@ module Ameba::AST

it "returns true if this is raise" do
node = as_node("raise e")
subject.flow_command?(node, false).should eq true
subject.flow_command?(node, false).should be_true
end

it "returns true if this is exit" do
node = as_node("exit")
subject.flow_command?(node, false).should eq true
subject.flow_command?(node, false).should be_true
end

it "returns true if this is abort" do
node = as_node("abort")
subject.flow_command?(node, false).should eq true
subject.flow_command?(node, false).should be_true
end

it "returns false otherwise" do
Expand All @@ -167,7 +167,7 @@ module Ameba::AST
describe "#flow_expression?" do
it "returns true if this is a flow command" do
node = as_node("return")
subject.flow_expression?(node, true).should eq true
subject.flow_expression?(node, true).should be_true
end

it "returns true if this is if-else consumed by flow expressions" do
Expand All @@ -178,7 +178,7 @@ module Ameba::AST
return :bar
end
CRYSTAL
subject.flow_expression?(node, false).should eq true
subject.flow_expression?(node, false).should be_true
end

it "returns true if this is unless-else consumed by flow expressions" do
Expand All @@ -189,7 +189,7 @@ module Ameba::AST
return :bar
end
CRYSTAL
subject.flow_expression?(node).should eq true
subject.flow_expression?(node).should be_true
end

it "returns true if this is case consumed by flow expressions" do
Expand All @@ -203,7 +203,7 @@ module Ameba::AST
return 3
end
CRYSTAL
subject.flow_expression?(node).should eq true
subject.flow_expression?(node).should be_true
end

it "returns true if this is exception handler consumed by flow expressions" do
Expand All @@ -214,7 +214,7 @@ module Ameba::AST
return e
end
CRYSTAL
subject.flow_expression?(node).should eq true
subject.flow_expression?(node).should be_true
end

it "returns true if this while consumed by flow expressions" do
Expand All @@ -223,7 +223,7 @@ module Ameba::AST
return
end
CRYSTAL
subject.flow_expression?(node).should eq true
subject.flow_expression?(node).should be_true
end

it "returns false if this while with break" do
Expand All @@ -241,7 +241,7 @@ module Ameba::AST
return
end
CRYSTAL
subject.flow_expression?(node).should eq true
subject.flow_expression?(node).should be_true
end

it "returns false if this until with break" do
Expand All @@ -259,7 +259,7 @@ module Ameba::AST
exp2
return
CRYSTAL
subject.flow_expression?(node).should eq true
subject.flow_expression?(node).should be_true
end

it "returns false otherwise" do
Expand All @@ -274,7 +274,7 @@ module Ameba::AST
describe "#raise?" do
it "returns true if this is a raise method call" do
node = as_node "raise e"
subject.raise?(node).should eq true
subject.raise?(node).should be_true
end

it "returns false if it has a receiver" do
Expand All @@ -291,12 +291,12 @@ module Ameba::AST
describe "#exit?" do
it "returns true if this is a exit method call" do
node = as_node "exit"
subject.exit?(node).should eq true
subject.exit?(node).should be_true
end

it "returns true if this is a exit method call with one argument" do
node = as_node "exit 1"
subject.exit?(node).should eq true
subject.exit?(node).should be_true
end

it "returns false if it has a receiver" do
Expand All @@ -313,17 +313,17 @@ module Ameba::AST
describe "#abort?" do
it "returns true if this is an abort method call" do
node = as_node "abort"
subject.abort?(node).should eq true
subject.abort?(node).should be_true
end

it "returns true if this is an abort method call with one argument" do
node = as_node "abort \"message\""
subject.abort?(node).should eq true
subject.abort?(node).should be_true
end

it "returns true if this is an abort method call with two arguments" do
node = as_node "abort \"message\", 1"
subject.abort?(node).should eq true
subject.abort?(node).should be_true
end

it "returns false if it has a receiver" do
Expand All @@ -340,7 +340,7 @@ module Ameba::AST
describe "#loop?" do
it "returns true if this is a loop method call" do
node = as_node "loop"
subject.loop?(node).should eq true
subject.loop?(node).should be_true
end

it "returns false if it has a receiver" do
Expand Down
3 changes: 2 additions & 1 deletion spec/ameba/ast/variabling/reference_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ module Ameba::AST
describe Reference do
it "is derived from a Variable" do
node = Crystal::Var.new "foo"
Reference.new(node, Scope.new as_node "foo = 1").is_a?(Variable).should be_true
ref = Reference.new(node, Scope.new as_node "foo = 1")
ref.should be_a Variable
end
end
end
4 changes: 2 additions & 2 deletions spec/ameba/cli/cmd_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ module Ameba::Cli

it "accepts --rules flag" do
c = Cli.parse_args %w[--rules]
c.rules?.should eq true
c.rules?.should be_true
end

it "defaults all? flag to false" do
Expand All @@ -76,7 +76,7 @@ module Ameba::Cli

it "accepts --all flag" do
c = Cli.parse_args %w[--all]
c.all?.should eq true
c.all?.should be_true
end

it "accepts --gen-config flag" do
Expand Down
5 changes: 4 additions & 1 deletion spec/ameba/formatter/disabled_formatter_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ module Ameba::Formatter
output = IO::Memory.new
subject = DisabledFormatter.new output

before_each do
output.clear
end

describe "#finished" do
it "writes a final message" do
subject.finished [Source.new ""]
Expand All @@ -24,7 +28,6 @@ module Ameba::Formatter
log.should contain "#{path}:1 #{ErrorRule.rule_name}"
log.should contain "#{path}:2 #{NamedRule.rule_name}"
ensure
output.clear
Colorize.enabled = true
end

Expand Down
33 changes: 19 additions & 14 deletions spec/ameba/formatter/explain_formatter_spec.cr
Original file line number Diff line number Diff line change
@@ -1,21 +1,24 @@
require "../../spec_helper"

module Ameba
def explanation(source)
output = IO::Memory.new
ErrorRule.new.catch(source)
location = {file: "source.cr", line: 1, column: 1}
Formatter::ExplainFormatter.new(output, location).finished([source])
output.to_s
end
private LOCATION = {file: "source.cr", line: 1, column: 1}

private def explanation(source)
Ameba::ErrorRule.new.catch(source)

describe Formatter::ExplainFormatter do
output = IO::Memory.new
Ameba::Formatter::ExplainFormatter.new(output, LOCATION).finished([source])
output.to_s
end

module Ameba::Formatter
describe ExplainFormatter do
describe "#location" do
it "returns crystal location" do
location = Formatter::ExplainFormatter
.new(STDOUT, {file: "compiler.cr", line: 3, column: 8}).location
location = ExplainFormatter
.new(STDOUT, {file: "compiler.cr", line: 3, column: 8})
.location

location.is_a?(Crystal::Location).should be_true
location.should be_a Crystal::Location
location.filename.should eq "compiler.cr"
location.line_number.should eq 3
location.column_number.should eq 8
Expand All @@ -24,8 +27,10 @@ module Ameba

describe "#output" do
it "returns io" do
output = Formatter::ExplainFormatter
.new(STDOUT, {file: "compiler.cr", line: 3, column: 8}).output
output = ExplainFormatter
.new(STDOUT, {file: "compiler.cr", line: 3, column: 8})
.output

output.should eq STDOUT
end
end
Expand Down
19 changes: 10 additions & 9 deletions spec/ameba/formatter/flycheck_formatter_spec.cr
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
require "../../spec_helper"

private def flycheck
output = IO::Memory.new
Ameba::Formatter::FlycheckFormatter.new output
end

module Ameba::Formatter
describe FlycheckFormatter do
output = IO::Memory.new
subject = FlycheckFormatter.new output

before_each do
output.clear
end

context "problems not found" do
it "reports nothing" do
subject = flycheck
subject.source_finished Source.new ""
subject.output.to_s.empty?.should be_true
end
Expand All @@ -19,7 +20,7 @@ module Ameba::Formatter
it "reports an issue" do
s = Source.new "a = 1", "source.cr"
s.add_issue DummyRule.new, {1, 2}, "message"
subject = flycheck

subject.source_finished s
subject.output.to_s.should eq(
"source.cr:1:2: C: [#{DummyRule.rule_name}] message\n"
Expand All @@ -29,7 +30,7 @@ module Ameba::Formatter
it "properly reports multi-line message" do
s = Source.new "a = 1", "source.cr"
s.add_issue DummyRule.new, {1, 2}, "multi\nline"
subject = flycheck

subject.source_finished s
subject.output.to_s.should eq(
"source.cr:1:2: C: [#{DummyRule.rule_name}] multi line\n"
Expand All @@ -39,7 +40,7 @@ module Ameba::Formatter
it "reports nothing if location was not set" do
s = Source.new "a = 1", "source.cr"
s.add_issue DummyRule.new, Crystal::Nop.new, "message"
subject = flycheck

subject.source_finished s
subject.output.to_s.should eq ""
end
Expand Down
15 changes: 12 additions & 3 deletions spec/ameba/formatter/github_actions_formatter_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ module Ameba::Formatter
source.add_issue DummyRule.new, location, location, "message\n2nd line"

subject.source_finished(source)
output.to_s.should eq("::notice file=/path/to/file.cr,line=1,col=2,endLine=1,endColumn=2,title=Ameba/DummyRule::message%0A2nd line\n")
output.to_s.should eq(
"::notice file=/path/to/file.cr,line=1,col=2,endLine=1,endColumn=2," \
"title=Ameba/DummyRule::message%0A2nd line\n"
)
end
end

Expand Down Expand Up @@ -78,8 +81,14 @@ module Ameba::Formatter
summary.should contain "### Issues found:"
summary.should contain "#### `src/source.cr` (**2** issues)"
if repo && sha
summary.should contain "| [1-2](https://github.com/#{repo}/blob/#{sha}/src/source.cr#L1-L2) | Convention | Ameba/DummyRule | DummyRuleError |"
summary.should contain "| [1](https://github.com/#{repo}/blob/#{sha}/src/source.cr#L1) | Convention | Ameba/DummyRule | DummyRuleError 2 |"
summary.should contain(
"| [1-2](https://github.com/#{repo}/blob/#{sha}/src/source.cr#L1-L2) " \
"| Convention | Ameba/DummyRule | DummyRuleError |"
)
summary.should contain(
"| [1](https://github.com/#{repo}/blob/#{sha}/src/source.cr#L1) " \
"| Convention | Ameba/DummyRule | DummyRuleError 2 |"
)
else
summary.should contain "| 1-2 | Convention | Ameba/DummyRule | DummyRuleError |"
summary.should contain "| 1 | Convention | Ameba/DummyRule | DummyRuleError 2 |"
Expand Down
Loading