Check for unneeded directives when all other rules are done

This commit is contained in:
Vitalii Elenhaupt 2018-02-02 22:11:18 +02:00 committed by V. Elenhaupt
parent 6fb483a2dd
commit eda5960b0f
8 changed files with 53 additions and 17 deletions

View file

@ -14,10 +14,6 @@ module Ameba::Rule
rules.should contain DummyRule rules.should contain DummyRule
rules.should contain NoProperties rules.should contain NoProperties
end end
it "should not include syntax rule" do
Rule.rules.should_not contain Rule::Syntax
end
end end
context "properties" do context "properties" do

View file

@ -66,14 +66,14 @@ module Ameba
config = Config.load config_sample config = Config.load config_sample
it "updates enabled property" do it "updates enabled property" do
name = DummyRule.class_name name = DummyRule.rule_name
config.update_rule name, enabled: false config.update_rule name, enabled: false
rule = config.rules.find(&.name.== name).not_nil! rule = config.rules.find(&.name.== name).not_nil!
rule.enabled.should be_false rule.enabled.should be_false
end end
it "updates excluded property" do it "updates excluded property" do
name = DummyRule.class_name name = DummyRule.rule_name
excluded = %w(spec/source.cr) excluded = %w(spec/source.cr)
config.update_rule name, excluded: excluded config.update_rule name, excluded: excluded
rule = config.rules.find(&.name.== name).not_nil! rule = config.rules.find(&.name.== name).not_nil!

View file

@ -44,7 +44,7 @@ module Ameba
end end
it "creates a todo with run details" do it "creates a todo with run details" do
create_todo.should contain "Run `ameba --only #{DummyRule.class_name}`" create_todo.should contain "Run `ameba --only #{DummyRule.rule_name}`"
end end
it "excludes source from this rule" do it "excludes source from this rule" do

View file

@ -67,7 +67,7 @@ module Ameba::Rule
it "fails if there is disabled UnneededDisableDirective" do it "fails if there is disabled UnneededDisableDirective" do
s = Source.new %Q( s = Source.new %Q(
# ameba:disable #{UnneededDisableDirective.class_name} # ameba:disable #{UnneededDisableDirective.rule_name}
a = 1 a = 1
), "source.cr" ), "source.cr"
s.error UnneededDisableDirective.new, 3, 1, "Alarm!", :disabled s.error UnneededDisableDirective.new, 3, 1, "Alarm!", :disabled

View file

@ -6,7 +6,7 @@ module Ameba
config.formatter = formatter config.formatter = formatter
config.files = files config.files = files
config.update_rule ErrorRule.class_name, enabled: false config.update_rule ErrorRule.rule_name, enabled: false
Runner.new(config) Runner.new(config)
end end
@ -59,7 +59,7 @@ module Ameba
Runner.new(rules, [source], formatter).run Runner.new(rules, [source], formatter).run
source.should_not be_valid source.should_not be_valid
source.errors.first.rule.name.should eq "Syntax" source.errors.first.rule.name.should eq Rule::Syntax.rule_name
end end
it "does not run other rules" do it "does not run other rules" do
@ -75,6 +75,19 @@ module Ameba
source.errors.size.should eq 1 source.errors.size.should eq 1
end end
end end
context "unneeded disables" do
it "reports an error if such disable exists" do
rules = [Rule::UnneededDisableDirective.new] of Rule::Base
source = Source.new %(
a = 1 # ameba:disable LineLength
)
Runner.new(rules, [source], formatter).run
source.should_not be_valid
source.errors.first.rule.name.should eq Rule::UnneededDisableDirective.rule_name
end
end
end end
describe "#success?" do describe "#success?" do

View file

@ -186,7 +186,7 @@ class Ameba::Config
def self.new(config = nil) def self.new(config = nil)
if (raw = config.try &.raw).is_a? Hash if (raw = config.try &.raw).is_a? Hash
yaml = raw[class_name]?.try &.to_yaml yaml = raw[rule_name]?.try &.to_yaml
end end
from_yaml yaml || "{}" from_yaml yaml || "{}"
end end

View file

@ -1,7 +1,9 @@
module Ameba::Rule module Ameba::Rule
# List of names of the special rules, which
# behave differently than usual rules.
SPECIAL = [ SPECIAL = [
Syntax.class_name, Syntax.rule_name,
UnneededDisableDirective.class_name, UnneededDisableDirective.rule_name,
] ]
# Represents a base of all rules. In other words, all rules # Represents a base of all rules. In other words, all rules
@ -60,7 +62,7 @@ module Ameba::Rule
# ``` # ```
# #
def name def name
{{@type}}.class_name {{@type}}.rule_name
end end
# Checks whether the source is excluded from this rule. # Checks whether the source is excluded from this rule.
@ -78,7 +80,18 @@ module Ameba::Rule
end end
end end
protected def self.class_name # Returns true if this rule is special and behaves differently than
# usual rules.
#
# ```
# my_rule.special? # => true or false
# ```
#
def special?
SPECIAL.includes? name
end
protected def self.rule_name
name.gsub("Ameba::Rule::", "") name.gsub("Ameba::Rule::", "")
end end
@ -95,6 +108,6 @@ module Ameba::Rule
# ``` # ```
# #
def self.rules def self.rules
Base.subclasses.reject! &.== Rule::Syntax Base.subclasses
end end
end end

View file

@ -23,6 +23,9 @@ module Ameba
# A syntax rule which always inspects a source first # A syntax rule which always inspects a source first
@syntax_rule = Rule::Syntax.new @syntax_rule = Rule::Syntax.new
# Checks for unneeded disable directives. Always inspects a source last
@unneeded_disable_directive_rule : Rule::Base?
# Instantiates a runner using a `config`. # Instantiates a runner using a `config`.
# #
# ``` # ```
@ -36,7 +39,11 @@ module Ameba
def initialize(config : Config) def initialize(config : Config)
@sources = load_sources(config) @sources = load_sources(config)
@formatter = config.formatter @formatter = config.formatter
@rules = config.rules.select &.enabled @rules = config.rules.select(&.enabled).reject!(&.special?)
@unneeded_disable_directive_rule =
config.rules
.find &.name.==(Rule::UnneededDisableDirective.rule_name)
end end
# :nodoc: # :nodoc:
@ -65,6 +72,7 @@ module Ameba
next if rule.excluded?(source) next if rule.excluded?(source)
rule.test(source) rule.test(source)
end end
check_unneeded_directives(source)
end end
@formatter.source_finished source @formatter.source_finished source
@ -87,6 +95,12 @@ module Ameba
@sources.all? &.valid? @sources.all? &.valid?
end end
private def check_unneeded_directives(source)
if (rule = @unneeded_disable_directive_rule) && rule.enabled
rule.test(source)
end
end
private def load_sources(config) private def load_sources(config)
config.files.map do |wildcard| config.files.map do |wildcard|
wildcard += "/**/*.cr" if File.directory?(wildcard) wildcard += "/**/*.cr" if File.directory?(wildcard)