Readability-related refactors

This commit is contained in:
Sijawusz Pur Rahnama 2022-11-14 01:24:29 +01:00
parent a6ebb48f14
commit e668ba5bf5
24 changed files with 89 additions and 61 deletions

View file

@ -191,7 +191,7 @@ module Ameba::AST
it "returns true if node is nested to Crystal::Macro" do
nodes = as_nodes %(
macro included
{{@type.each do |type| a = type end}}
{{ @type.each do |type| a = type end }}
end
)
outer_scope = Scope.new nodes.macro_nodes.first

View file

@ -40,7 +40,9 @@ module Ameba
---
Globs: 100
CONFIG
expect_raises(Exception, "incorrect 'Globs' section in a config file") { Config.new(yml) }
expect_raises(Exception, "Incorrect 'Globs' section in a config file") do
Config.new(yml)
end
end
it "initializes excluded as string" do
@ -68,7 +70,9 @@ module Ameba
---
Excluded: true
CONFIG
expect_raises(Exception, "incorrect 'Excluded' section in a config file") { Config.new(yml) }
expect_raises(Exception, "Incorrect 'Excluded' section in a config file") do
Config.new(yml)
end
end
end

View file

@ -211,11 +211,13 @@ module Ameba::Rule::Lint
end
def bar
{{@type.instance_vars.map do |ivar|
{{
@type.instance_vars.map do |ivar|
ivar.annotations(Name).each do |ann|
puts ann.args
end
end}}
end
}}
end
end
end

View file

@ -218,11 +218,11 @@ module Ameba::Rule::Lint
s = Source.new %(
record X do
macro foo(a, b)
{{a}} + {{b}}
{{ a }} + {{ b }}
end
macro bar(a, b, c)
{{a}} + {{b}} + {{c}}
{{ a }} + {{ b }} + {{ c }}
end
end
)

View file

@ -949,7 +949,7 @@ module Ameba::Rule::Lint
foo = 22
{% for x in %w(foo) %}
add({{x.id}})
add({{ x.id }})
{% end %}
)
subject.catch(s).should be_valid

View file

@ -190,7 +190,7 @@ module Ameba
def test(source, node : Crystal::ClassDef)
return unless location = node.location
end_location = location.adjust(column_number: {{"class".size - 1}})
end_location = location.adjust(column_number: {{ "class".size - 1 }})
issue_for(location, end_location, message: "class to module") do |corrector|
corrector.replace(location, end_location, "module")
@ -208,7 +208,7 @@ module Ameba
def test(source, node : Crystal::ModuleDef)
return unless location = node.location
end_location = location.adjust(column_number: {{"module".size - 1}})
end_location = location.adjust(column_number: {{ "module".size - 1 }})
issue_for(location, end_location, message: "module to class") do |corrector|
corrector.replace(location, end_location, "class")
@ -265,12 +265,12 @@ module Ameba
end
{% for node in NODES %}
{{getter_name = node.stringify.split("::").last.underscore + "_nodes"}}
{{ getter_name = node.stringify.split("::").last.underscore + "_nodes" }}
getter {{getter_name.id}} = [] of {{node}}
getter {{ getter_name.id }} = [] of {{ node }}
def visit(node : {{node}})
{{getter_name.id}} << node
def visit(node : {{ node }})
{{ getter_name.id }} << node
true
end
{% end %}

View file

@ -58,7 +58,9 @@ module Ameba::AST
control_flow_found ||= !loop?(exp) && flow_expression?(exp, in_loop?)
end
when Crystal::BinaryOp
unreachable_nodes << current_node.right if flow_expression?(current_node.left, in_loop?)
if flow_expression?(current_node.left, in_loop?)
unreachable_nodes << current_node.right
end
end
unreachable_nodes

View file

@ -106,6 +106,7 @@ module Ameba::AST
# ```
def spawn_block?
return false unless node.is_a?(Crystal::Block)
call = node.as(Crystal::Block).call
call.try(&.name) == "spawn"
end

View file

@ -32,7 +32,6 @@ module Ameba::AST::Util
# to determine and cut a piece of source of the node.
def node_source(node, code_lines)
loc, end_loc = node.location, node.end_location
return unless loc && end_loc
source_between(loc, end_loc, code_lines)
@ -46,7 +45,7 @@ module Ameba::AST::Util
first_line, last_line = node_lines[0]?, node_lines[-1]?
return if first_line.nil? || last_line.nil?
return if first_line.size < column # compiler reports incorrection location
return if first_line.size < column # compiler reports incorrect location
node_lines[0] = first_line.sub(0...column, "")

View file

@ -39,10 +39,10 @@ module Ameba::AST
# Name of the argument.
def name
case current_node = node
when Crystal::Var then current_node.name
when Crystal::Arg then current_node.name
when Crystal::Var, Crystal::Arg
current_node.name
else
raise ArgumentError.new "invalid node"
raise ArgumentError.new "Invalid node"
end
end
end

View file

@ -97,6 +97,7 @@ module Ameba::AST
return false unless (assign = node).is_a?(Crystal::Assign)
return false unless (value = assign.value).is_a?(Crystal::Call)
return false unless (obj = value.obj).is_a?(Crystal::Var)
obj.name.starts_with? "__arg"
end
end

View file

@ -1,8 +1,8 @@
module Ameba::AST
# Represents the existence of the local variable.
# Holds the var node and variable assigments.
# Holds the var node and variable assignments.
class Variable
# List of the assigments of this variable.
# List of the assignments of this variable.
getter assignments = [] of Assignment
# List of the references of this variable.
@ -30,7 +30,7 @@ module Ameba::AST
def initialize(@node, @scope)
end
# Returns true if it is a special variable, i.e `$?`.
# Returns `true` if it is a special variable, i.e `$?`.
def special?
@node.special_var?
end
@ -50,7 +50,7 @@ module Ameba::AST
update_assign_reference!
end
# Returns true if variable has any reference.
# Returns `true` if variable has any reference.
#
# ```
# variable = Variable.new(node, scope)
@ -93,7 +93,7 @@ module Ameba::AST
end
end
# Returns true if the current var is referenced in
# Returns `true` if the current var is referenced in
# in the block. For example this variable is captured
# by block:
#
@ -110,26 +110,29 @@ module Ameba::AST
# ```
def captured_by_block?(scope = @scope)
scope.inner_scopes.each do |inner_scope|
return true if inner_scope.block? && inner_scope.references?(self, check_inner_scopes: false)
return true if inner_scope.block? &&
inner_scope.references?(self, check_inner_scopes: false)
return true if captured_by_block?(inner_scope)
end
false
end
# Returns true if current variable potentially referenced in a macro,
# false if not.
# Returns `true` if current variable potentially referenced in a macro,
# `false` if not.
def used_in_macro?(scope = @scope)
scope.inner_scopes.each do |inner_scope|
return true if MacroReferenceFinder.new(inner_scope.node, node.name).references
end
return true if MacroReferenceFinder.new(scope.node, node.name).references
return true if (outer_scope = scope.outer_scope) && used_in_macro?(outer_scope)
return true if (outer_scope = scope.outer_scope) &&
used_in_macro?(outer_scope)
false
end
# Returns true if the variable is a target (on the left) of the assignment,
# false otherwise.
# Returns `true` if the variable is a target (on the left) of the assignment,
# `false` otherwise.
def target_of?(assign)
case assign
when Crystal::Assign then eql?(assign.target)
@ -141,12 +144,12 @@ module Ameba::AST
end
end
# Returns true if the name starts with '_', false if not.
# Returns `true` if the name starts with '_', `false` if not.
def ignored?
name.starts_with? '_'
end
# Returns true if the `node` represents exactly
# Returns `true` if the `node` represents exactly
# the same Crystal node as `@node`.
def eql?(node)
node.is_a?(Crystal::Var) &&
@ -154,7 +157,7 @@ module Ameba::AST
node.location == @node.location
end
# Returns true if the variable is declared before the `node`.
# Returns `true` if the variable is declared before the `node`.
def declared_before?(node)
var_location, node_location = location, node.location

View file

@ -2,6 +2,7 @@ module Ameba::AST
# AST Visitor that counts occurrences of certain keywords
class CountingVisitor < Crystal::Visitor
DEFAULT_COMPLEXITY = 1
getter macro_condition = false
# Creates a new counting visitor
@ -44,6 +45,7 @@ module Ameba::AST
def visit(node : Crystal::MacroIf | Crystal::MacroFor)
@macro_condition = true
@complexity = DEFAULT_COMPLEXITY
false
end
end

View file

@ -45,11 +45,11 @@ module Ameba::AST
end
{% for name in NODES %}
# A visit callback for `Crystal::{{name}}` node.
# A visit callback for `Crystal::{{ name }}` node.
#
# Returns `true` if the child nodes should be traversed as well,
# `false` otherwise.
def visit(node : Crystal::{{name}})
def visit(node : Crystal::{{ name }})
return false if skip?(node)
@rule.test @source, node

View file

@ -48,7 +48,8 @@ module Ameba::AST
end
private def on_assign_end(target, node)
target.is_a?(Crystal::Var) && @current_scope.assign_variable(target.name, node)
target.is_a?(Crystal::Var) &&
@current_scope.assign_variable(target.name, node)
end
# :nodoc:
@ -58,7 +59,7 @@ module Ameba::AST
{% for name in NODES %}
# :nodoc:
def visit(node : Crystal::{{name}})
def visit(node : Crystal::{{ name }})
on_scope_enter(node)
end
{% end %}
@ -96,13 +97,15 @@ module Ameba::AST
# :nodoc:
def visit(node : Crystal::TypeDeclaration)
return if @current_scope.type_definition? || !(var = node.var).is_a?(Crystal::Var)
@current_scope.add_variable var
return if @current_scope.type_definition?
return if !(var = node.var).is_a?(Crystal::Var)
@current_scope.add_variable(var)
end
# :nodoc:
def visit(node : Crystal::Arg)
@current_scope.add_argument node
@current_scope.add_argument(node)
end
# :nodoc:

View file

@ -123,10 +123,10 @@ class Ameba::Config
# config.formatter = :progress
# ```
def formatter=(name : String | Symbol)
unless f = AVAILABLE_FORMATTERS[name]?
unless formatter = AVAILABLE_FORMATTERS[name]?
raise "Unknown formatter `#{name}`. Use one of #{Config.formatter_names}."
end
@formatter = f.new
@formatter = formatter.new
end
# Updates rule properties.
@ -180,7 +180,7 @@ class Ameba::Config
when .as_s? then [value.to_s]
when .as_a? then value.as_a.map(&.as_s)
else
raise "incorrect '#{section_name}' section in a config files"
raise "Incorrect '#{section_name}' section in a config files"
end
end
@ -241,8 +241,8 @@ class Ameba::Config
{% properties[name] = {key: key, default: value, type: type, converter: converter} %}
@[YAML::Field(key: {{key}}, converter: {{converter}}, type: {{type}})]
property {{name}} : {{type}} = {{value}}
@[YAML::Field(key: {{ key }}, converter: {{ converter }}, type: {{ type }})]
property {{ name }} : {{ type }} = {{ value }}
{% end %}
{% if properties["enabled".id] == nil %}
@ -253,7 +253,7 @@ class Ameba::Config
{% if properties["severity".id] == nil %}
{% default = @type.name.starts_with?("Ameba::Rule::Lint") ? "Ameba::Severity::Warning".id : "Ameba::Severity::Convention".id %}
@[YAML::Field(key: "Severity", converter: Ameba::SeverityYamlConverter)]
property severity = {{default}}
property severity = {{ default }}
{% end %}
{% if properties["excluded".id] == nil %}

View file

@ -1,7 +1,8 @@
module Ameba
# A module that utilizes inline comments parsing and processing logic.
module InlineComments
COMMENT_DIRECTIVE_REGEX = /# ameba:(?<action>\w+) (?<rules>\w+(?:\/\w+)?(?:,? \w+(?:\/\w+)?)*)/
COMMENT_DIRECTIVE_REGEX =
/# ameba:(?<action>\w+) (?<rules>\w+(?:\/\w+)?(?:,? \w+(?:\/\w+)?)*)/
# Available actions in the inline comments
enum Action
@ -87,15 +88,17 @@ module Ameba
return false unless directive = parse_inline_directive(line)
return false unless Action.parse?(directive[:action]).try(&.disable?)
directive[:rules].includes?(rule.name) ||
directive[:rules].includes?(rule.group)
rules = directive[:rules]
rules.includes?(rule.name) || rules.includes?(rule.group)
end
private def commented_out?(line)
commented = false
lexer = Crystal::Lexer.new(line).tap(&.comments_enabled = true)
Tokenizer.new(lexer).run { |t| commented = true if t.type.comment? }
Tokenizer.new(lexer).run do |token|
commented = true if token.type.comment?
end
commented
end
end

View file

@ -64,7 +64,7 @@ module Ameba::Rule
# MyRule.new.name # => "MyRule"
# ```
def name
{{@type}}.rule_name
{{ @type }}.rule_name
end
# Returns a group this rule belong to.
@ -77,7 +77,7 @@ module Ameba::Rule
# MyGroup::MyRule.new.group # => "MyGroup"
# ```
def group
{{@type}}.group_name
{{ @type }}.group_name
end
# Checks whether the source is excluded from this rule.
@ -113,7 +113,7 @@ module Ameba::Rule
end
macro issue_for(*args, **kwargs, &block)
source.add_issue(self, {{*args}}, {{**kwargs}}) {{block}}
source.add_issue(self, {{ *args }}, {{ **kwargs }}) {{ block }}
end
protected def self.rule_name

View file

@ -1,5 +1,5 @@
module Ameba::Rule::Layout
# A rule that disallows trailing whitespaces.
# A rule that disallows trailing whitespace.
#
# YAML configuration example:
#

View file

@ -54,7 +54,7 @@ module Ameba::Rule::Lint
issue_for name_location, end_location, msg do |corrector|
corrector.insert_after(name_location_end, '!')
corrector.remove_trailing(node, ".not_nil!".size)
corrector.remove_trailing(node, {{ ".not_nil!".size }})
end
end
end

View file

@ -114,7 +114,7 @@ module Ameba::Rule::Style
if node.else.is_a?(Crystal::Nop)
return unless end_end_loc = node.end_location
end_loc = end_end_loc.adjust(column_number: {{1 - "end".size}})
end_loc = end_end_loc.adjust(column_number: {{ 1 - "end".size }})
issue_for keyword_loc, keyword_end_loc, MSG % example do |corrector|
replacement = "#{scope_exiting_keyword} #{conditional_keyword}"

View file

@ -87,6 +87,7 @@ module Ameba
# ```
def run
@formatter.started @sources
channels = @sources.map { Channel(Exception?).new }
@sources.each_with_index do |source, idx|
channel = channels[idx]
@ -219,7 +220,9 @@ module Ameba
end
private def check_unneeded_directives(source)
return unless (rule = @unneeded_disable_directive_rule) && rule.enabled
return unless rule = @unneeded_disable_directive_rule
return unless rule.enabled
rule.test(source)
end
end

View file

@ -36,7 +36,7 @@ module Ameba
true
end
# Returns lines of code splitted by new line character.
# Returns lines of code split by new line character.
# Since `code` is immutable and can't be changed, this
# method caches lines in an instance variable, so calling
# it second time will not perform a split, but will return

View file

@ -32,10 +32,12 @@ class Ameba::Source::Rewriter
def empty?
replacement = @replacement
@insert_before.empty? &&
@insert_after.empty? &&
@children.empty? &&
(replacement.nil? || (replacement.empty? && @begin_pos == @end_pos))
(replacement.nil? ||
(replacement.empty? && @begin_pos == @end_pos))
end
def ordered_replacements
@ -50,7 +52,10 @@ class Ameba::Source::Rewriter
def insertion?
replacement = @replacement
!@insert_before.empty? || !@insert_after.empty? || (replacement && !replacement.empty?)
!@insert_before.empty? ||
!@insert_after.empty? ||
(replacement && !replacement.empty?)
end
protected def with(*,