Misc cleanups/refactors
This commit is contained in:
parent
547fec5a94
commit
f3f1f3a2ab
|
@ -4,4 +4,4 @@ Documentation/DocumentationAdmonition:
|
||||||
|
|
||||||
Lint/Typos:
|
Lint/Typos:
|
||||||
Excluded:
|
Excluded:
|
||||||
- spec/ameba/rule/lint/typos_spec.cr
|
- spec/ameba/rule/lint/typos_spec.cr
|
||||||
|
|
|
@ -186,8 +186,8 @@ Excluded:
|
||||||
``` yaml
|
``` yaml
|
||||||
Style/RedundantBegin:
|
Style/RedundantBegin:
|
||||||
Excluded:
|
Excluded:
|
||||||
- src/server/processor.cr
|
- src/server/processor.cr
|
||||||
- src/server/api.cr
|
- src/server/api.cr
|
||||||
```
|
```
|
||||||
|
|
||||||
### Rules
|
### Rules
|
||||||
|
|
|
@ -9,9 +9,9 @@ targets:
|
||||||
main: src/cli.cr
|
main: src/cli.cr
|
||||||
|
|
||||||
scripts:
|
scripts:
|
||||||
# TODO: remove pre-compiled executable in future releases
|
|
||||||
postinstall: shards build -Dpreview_mt
|
postinstall: shards build -Dpreview_mt
|
||||||
|
|
||||||
|
# TODO: remove pre-compiled executable in future releases
|
||||||
executables:
|
executables:
|
||||||
- ameba
|
- ameba
|
||||||
- ameba.cr
|
- ameba.cr
|
||||||
|
|
|
@ -4,16 +4,16 @@ module Ameba
|
||||||
subject = Rule::Lint::EmptyExpression.new
|
subject = Rule::Lint::EmptyExpression.new
|
||||||
|
|
||||||
private def it_detects_empty_expression(code, *, file = __FILE__, line = __LINE__)
|
private def it_detects_empty_expression(code, *, file = __FILE__, line = __LINE__)
|
||||||
it %(detects empty expression "#{code}"), file, line do
|
it "detects empty expression #{code.inspect}", file, line do
|
||||||
s = Source.new code
|
source = Source.new code
|
||||||
rule = Rule::Lint::EmptyExpression.new
|
rule = Rule::Lint::EmptyExpression.new
|
||||||
rule.catch(s).should_not be_valid, file: file, line: line
|
rule.catch(source).should_not be_valid, file: file, line: line
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe Rule::Lint::EmptyExpression do
|
describe Rule::Lint::EmptyExpression do
|
||||||
it "passes if there is no empty expression" do
|
it "passes if there is no empty expression" do
|
||||||
s = Source.new <<-CRYSTAL
|
expect_no_issues subject, <<-CRYSTAL
|
||||||
def method()
|
def method()
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -31,7 +31,6 @@ module Ameba
|
||||||
begin "" end
|
begin "" end
|
||||||
[nil] << nil
|
[nil] << nil
|
||||||
CRYSTAL
|
CRYSTAL
|
||||||
subject.catch(s).should be_valid
|
|
||||||
end
|
end
|
||||||
|
|
||||||
it_detects_empty_expression %(())
|
it_detects_empty_expression %(())
|
||||||
|
@ -91,10 +90,10 @@ module Ameba
|
||||||
)
|
)
|
||||||
|
|
||||||
it "does not report empty expression in macro" do
|
it "does not report empty expression in macro" do
|
||||||
s = Source.new %q(
|
expect_no_issues subject, <<-CRYSTAL
|
||||||
module MyModule
|
module MyModule
|
||||||
macro conditional_error_for_inline_callbacks
|
macro conditional_error_for_inline_callbacks
|
||||||
\{%
|
\\{%
|
||||||
raise ""
|
raise ""
|
||||||
%}
|
%}
|
||||||
end
|
end
|
||||||
|
@ -102,8 +101,7 @@ module Ameba
|
||||||
macro before_save(x = nil)
|
macro before_save(x = nil)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
)
|
CRYSTAL
|
||||||
subject.catch(s).should be_valid
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -19,12 +19,15 @@ module Ameba::Rule::Lint
|
||||||
# ^^^^^^^^^ error: Typo found: arugments -> arguments
|
# ^^^^^^^^^ error: Typo found: arugments -> arguments
|
||||||
def tpos
|
def tpos
|
||||||
# ^^^^ error: Typo found: tpos -> typos
|
# ^^^^ error: Typo found: tpos -> typos
|
||||||
|
:otput
|
||||||
|
# ^^^^^ error: Typo found: otput -> output
|
||||||
end
|
end
|
||||||
CRYSTAL
|
CRYSTAL
|
||||||
|
|
||||||
expect_correction source, <<-CRYSTAL
|
expect_correction source, <<-CRYSTAL
|
||||||
# method with no arguments
|
# method with no arguments
|
||||||
def typos
|
def typos
|
||||||
|
:output
|
||||||
end
|
end
|
||||||
CRYSTAL
|
CRYSTAL
|
||||||
end
|
end
|
||||||
|
|
|
@ -123,7 +123,7 @@ module Ameba
|
||||||
it "#int_min_digits" do
|
it "#int_min_digits" do
|
||||||
rule = Rule::Style::LargeNumbers.new
|
rule = Rule::Style::LargeNumbers.new
|
||||||
rule.int_min_digits = 10
|
rule.int_min_digits = 10
|
||||||
expect_no_issues rule, %q(1200000)
|
expect_no_issues rule, "1200000"
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -27,11 +27,11 @@ module Ameba::Rule::Lint
|
||||||
description "Identifies usage of `index/rindex/find/match` calls followed by `not_nil!`"
|
description "Identifies usage of `index/rindex/find/match` calls followed by `not_nil!`"
|
||||||
end
|
end
|
||||||
|
|
||||||
|
MSG = "Use `%s! {...}` instead of `%s {...}.not_nil!`"
|
||||||
|
|
||||||
BLOCK_CALL_NAMES = %w(index rindex find)
|
BLOCK_CALL_NAMES = %w(index rindex find)
|
||||||
CALL_NAMES = %w(index rindex match)
|
CALL_NAMES = %w(index rindex match)
|
||||||
|
|
||||||
MSG = "Use `%s! {...}` instead of `%s {...}.not_nil!`"
|
|
||||||
|
|
||||||
def test(source)
|
def test(source)
|
||||||
AST::NodeVisitor.new self, source, skip: :macro
|
AST::NodeVisitor.new self, source, skip: :macro
|
||||||
end
|
end
|
||||||
|
|
|
@ -42,7 +42,7 @@ module Ameba::Rule::Lint
|
||||||
start_token = token.dup
|
start_token = token.dup
|
||||||
when .string?
|
when .string?
|
||||||
if (_start = start_token) && !issue
|
if (_start = start_token) && !issue
|
||||||
issue = array_entry_invalid?(token.value, _start.raw)
|
issue = array_entry_invalid?(token.value.to_s, _start.raw)
|
||||||
end
|
end
|
||||||
when .string_array_end?
|
when .string_array_end?
|
||||||
if (_start = start_token) && (_issue = issue)
|
if (_start = start_token) && (_issue = issue)
|
||||||
|
@ -63,7 +63,7 @@ module Ameba::Rule::Lint
|
||||||
end
|
end
|
||||||
|
|
||||||
private def check_array_entry(entry, symbols, literal)
|
private def check_array_entry(entry, symbols, literal)
|
||||||
MSG % {symbols, literal} if entry =~ /[#{symbols}]/
|
MSG % {symbols, literal} if entry.matches?(/[#{Regex.escape(symbols)}]/)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -15,7 +15,7 @@ module Ameba::Rule::Lint
|
||||||
class Typos < Base
|
class Typos < Base
|
||||||
properties do
|
properties do
|
||||||
description "Reports typos found in source files"
|
description "Reports typos found in source files"
|
||||||
bin_path nil.as(String?)
|
bin_path : String? = nil
|
||||||
fail_on_error false
|
fail_on_error false
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -56,7 +56,8 @@ module Ameba::Rule::Lint
|
||||||
location = block_arg.node.location
|
location = block_arg.node.location
|
||||||
end_location = location.try &.adjust(column_number: block_arg.name.size - 1)
|
end_location = location.try &.adjust(column_number: block_arg.name.size - 1)
|
||||||
|
|
||||||
if scope.yields?
|
case
|
||||||
|
when scope.yields?
|
||||||
if location && end_location
|
if location && end_location
|
||||||
issue_for location, end_location, MSG_YIELDED do |corrector|
|
issue_for location, end_location, MSG_YIELDED do |corrector|
|
||||||
corrector.remove(location, end_location)
|
corrector.remove(location, end_location)
|
||||||
|
@ -64,8 +65,7 @@ module Ameba::Rule::Lint
|
||||||
else
|
else
|
||||||
issue_for block_arg.node, MSG_YIELDED
|
issue_for block_arg.node, MSG_YIELDED
|
||||||
end
|
end
|
||||||
else
|
when !block_arg.ignored?
|
||||||
return if block_arg.ignored?
|
|
||||||
if location && end_location
|
if location && end_location
|
||||||
issue_for location, end_location, MSG_UNUSED % block_arg.name do |corrector|
|
issue_for location, end_location, MSG_UNUSED % block_arg.name do |corrector|
|
||||||
corrector.insert_before(location, '_')
|
corrector.insert_before(location, '_')
|
||||||
|
|
|
@ -30,7 +30,8 @@ module Ameba::Rule::Naming
|
||||||
|
|
||||||
def test(source, node : Crystal::Assign)
|
def test(source, node : Crystal::Assign)
|
||||||
return unless (target = node.target).is_a?(Crystal::Path)
|
return unless (target = node.target).is_a?(Crystal::Path)
|
||||||
name = target.names.first
|
|
||||||
|
name = target.to_s
|
||||||
expected = name.upcase
|
expected = name.upcase
|
||||||
|
|
||||||
return if name.in?(expected, name.camelcase)
|
return if name.in?(expected, name.camelcase)
|
||||||
|
|
|
@ -45,9 +45,11 @@ module Ameba::Rule::Naming
|
||||||
MSG = "Method name should be underscore-cased: %s, not %s"
|
MSG = "Method name should be underscore-cased: %s, not %s"
|
||||||
|
|
||||||
def test(source, node : Crystal::Def)
|
def test(source, node : Crystal::Def)
|
||||||
return if (expected = node.name.underscore) == node.name
|
name = node.name.to_s
|
||||||
|
|
||||||
issue_for node, MSG % {expected, node.name}, prefer_name_location: true
|
return if (expected = name.underscore) == name
|
||||||
|
|
||||||
|
issue_for node, MSG % {expected, name}, prefer_name_location: true
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -48,9 +48,7 @@ module Ameba::Rule::Naming
|
||||||
.select!(&.name.in?(CALL_NAMES))
|
.select!(&.name.in?(CALL_NAMES))
|
||||||
end
|
end
|
||||||
|
|
||||||
return unless calls
|
calls.try &.each do |exp|
|
||||||
|
|
||||||
calls.each do |exp|
|
|
||||||
exp.args.each do |arg|
|
exp.args.each do |arg|
|
||||||
name_node, is_bool =
|
name_node, is_bool =
|
||||||
case arg
|
case arg
|
||||||
|
|
|
@ -60,8 +60,8 @@ module Ameba::Rule::Naming
|
||||||
|
|
||||||
def test(source, node : Crystal::Alias | Crystal::ClassDef | Crystal::ModuleDef | Crystal::LibDef | Crystal::EnumDef)
|
def test(source, node : Crystal::Alias | Crystal::ClassDef | Crystal::ModuleDef | Crystal::LibDef | Crystal::EnumDef)
|
||||||
name = node.name.to_s
|
name = node.name.to_s
|
||||||
expected = name.camelcase
|
|
||||||
return if name == expected
|
return if (expected = name.camelcase) == name
|
||||||
|
|
||||||
issue_for node.name, MSG % {expected, name}
|
issue_for node.name, MSG % {expected, name}
|
||||||
end
|
end
|
||||||
|
|
|
@ -35,8 +35,8 @@ module Ameba::Rule::Naming
|
||||||
|
|
||||||
def test(source, node : Crystal::Var | Crystal::InstanceVar | Crystal::ClassVar)
|
def test(source, node : Crystal::Var | Crystal::InstanceVar | Crystal::ClassVar)
|
||||||
name = node.name.to_s
|
name = node.name.to_s
|
||||||
expected = name.underscore
|
|
||||||
return if name == expected
|
return if (expected = name.underscore) == name
|
||||||
|
|
||||||
issue_for node, MSG % {expected, name}
|
issue_for node, MSG % {expected, name}
|
||||||
end
|
end
|
||||||
|
|
|
@ -48,7 +48,9 @@ module Ameba::Rule::Performance
|
||||||
call_names %w(uniq sort sort_by shuffle reverse)
|
call_names %w(uniq sort sort_by shuffle reverse)
|
||||||
end
|
end
|
||||||
|
|
||||||
# All these methods are allocating a new object
|
MSG = "Use bang method variant `%s!` after chained `%s` call"
|
||||||
|
|
||||||
|
# All these methods allocate a new object
|
||||||
ALLOCATING_METHOD_NAMES = %w(
|
ALLOCATING_METHOD_NAMES = %w(
|
||||||
keys values values_at map map_with_index flat_map compact_map
|
keys values values_at map map_with_index flat_map compact_map
|
||||||
flatten compact select reject sample group_by chunks tally merge
|
flatten compact select reject sample group_by chunks tally merge
|
||||||
|
@ -56,8 +58,6 @@ module Ameba::Rule::Performance
|
||||||
transpose invert chars captures named_captures clone
|
transpose invert chars captures named_captures clone
|
||||||
)
|
)
|
||||||
|
|
||||||
MSG = "Use bang method variant `%s!` after chained `%s` call"
|
|
||||||
|
|
||||||
def test(source)
|
def test(source)
|
||||||
AST::NodeVisitor.new self, source, skip: :macro
|
AST::NodeVisitor.new self, source, skip: :macro
|
||||||
end
|
end
|
||||||
|
|
|
@ -29,9 +29,9 @@ module Ameba::Rule::Performance
|
||||||
description "Identifies usage of `sum/product` calls that follow `map`"
|
description "Identifies usage of `sum/product` calls that follow `map`"
|
||||||
end
|
end
|
||||||
|
|
||||||
|
MSG = "Use `%s {...}` instead of `map {...}.%s`"
|
||||||
|
|
||||||
CALL_NAMES = %w(sum product)
|
CALL_NAMES = %w(sum product)
|
||||||
MAP_NAME = "map"
|
|
||||||
MSG = "Use `%s {...}` instead of `map {...}.%s`"
|
|
||||||
|
|
||||||
def test(source)
|
def test(source)
|
||||||
AST::NodeVisitor.new self, source, skip: :macro
|
AST::NodeVisitor.new self, source, skip: :macro
|
||||||
|
@ -40,7 +40,7 @@ module Ameba::Rule::Performance
|
||||||
def test(source, node : Crystal::Call)
|
def test(source, node : Crystal::Call)
|
||||||
return unless node.name.in?(CALL_NAMES) && (obj = node.obj)
|
return unless node.name.in?(CALL_NAMES) && (obj = node.obj)
|
||||||
return unless obj.is_a?(Crystal::Call) && obj.block
|
return unless obj.is_a?(Crystal::Call) && obj.block
|
||||||
return unless obj.name == MAP_NAME
|
return unless obj.name == "map"
|
||||||
|
|
||||||
issue_for name_location(obj), name_end_location(node),
|
issue_for name_location(obj), name_end_location(node),
|
||||||
MSG % {node.name, node.name}
|
MSG % {node.name, node.name}
|
||||||
|
|
|
@ -47,8 +47,9 @@ module Ameba::Rule::Style
|
||||||
end
|
end
|
||||||
|
|
||||||
MSG = "Use `%s` instead of `%s`"
|
MSG = "Use `%s` instead of `%s`"
|
||||||
NEW = "%s(%s)"
|
|
||||||
OLD = "%s {...}"
|
OLD = "%s {...}"
|
||||||
|
NEW = "%s(%s)"
|
||||||
|
|
||||||
def test(source)
|
def test(source)
|
||||||
AST::NodeVisitor.new self, source, skip: :macro
|
AST::NodeVisitor.new self, source, skip: :macro
|
||||||
|
|
|
@ -227,9 +227,6 @@ module Ameba::Rule::Style
|
||||||
|
|
||||||
arg = block.args.first
|
arg = block.args.first
|
||||||
|
|
||||||
# we skip auto-generated blocks - `(1..3).any?(&.odd?)`
|
|
||||||
return if arg.name.starts_with?("__arg")
|
|
||||||
|
|
||||||
# we filter out the blocks that are of call type - `i.to_i64.odd?`
|
# we filter out the blocks that are of call type - `i.to_i64.odd?`
|
||||||
return unless (body = block.body).is_a?(Crystal::Call)
|
return unless (body = block.body).is_a?(Crystal::Call)
|
||||||
|
|
||||||
|
|
|
@ -67,7 +67,7 @@ module Ameba
|
||||||
|
|
||||||
@unneeded_disable_directive_rule =
|
@unneeded_disable_directive_rule =
|
||||||
config.rules
|
config.rules
|
||||||
.find &.name.==(Rule::Lint::UnneededDisableDirective.rule_name)
|
.find &.class.==(Rule::Lint::UnneededDisableDirective)
|
||||||
end
|
end
|
||||||
|
|
||||||
protected def initialize(@rules, @sources, @formatter, @severity, @autocorrect = false)
|
protected def initialize(@rules, @sources, @formatter, @severity, @autocorrect = false)
|
||||||
|
|
|
@ -74,7 +74,7 @@ module Ameba
|
||||||
|
|
||||||
# Returns `true` if *filepath* matches the source's path, `false` otherwise.
|
# Returns `true` if *filepath* matches the source's path, `false` otherwise.
|
||||||
def matches_path?(filepath)
|
def matches_path?(filepath)
|
||||||
path.in?(filepath, File.expand_path(filepath))
|
fullpath == File.expand_path(filepath)
|
||||||
end
|
end
|
||||||
|
|
||||||
# Converts an AST location to a string position.
|
# Converts an AST location to a string position.
|
||||||
|
|
|
@ -72,12 +72,15 @@ class Ameba::Source
|
||||||
|
|
||||||
# Replaces the code of the given range with *content*.
|
# Replaces the code of the given range with *content*.
|
||||||
def replace(begin_pos, end_pos, content)
|
def replace(begin_pos, end_pos, content)
|
||||||
combine(begin_pos, end_pos, replacement: content.to_s)
|
combine begin_pos, end_pos,
|
||||||
|
replacement: content.to_s
|
||||||
end
|
end
|
||||||
|
|
||||||
# Inserts the given strings before and after the given range.
|
# Inserts the given strings before and after the given range.
|
||||||
def wrap(begin_pos, end_pos, insert_before, insert_after)
|
def wrap(begin_pos, end_pos, insert_before, insert_after)
|
||||||
combine(begin_pos, end_pos, insert_before: insert_before.to_s, insert_after: insert_after.to_s)
|
combine begin_pos, end_pos,
|
||||||
|
insert_before: insert_before.to_s,
|
||||||
|
insert_after: insert_after.to_s
|
||||||
end
|
end
|
||||||
|
|
||||||
# Shortcut for `replace(begin_pos, end_pos, "")`
|
# Shortcut for `replace(begin_pos, end_pos, "")`
|
||||||
|
|
Loading…
Reference in New Issue