Skip to content

Commit

Permalink
Check for unneeded directives when all other rules are done
Browse files Browse the repository at this point in the history
  • Loading branch information
veelenga committed Feb 2, 2018
1 parent 6fb483a commit eda5960
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 17 deletions.
4 changes: 0 additions & 4 deletions spec/ameba/base_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ module Ameba::Rule
rules.should contain DummyRule
rules.should contain NoProperties
end

it "should not include syntax rule" do
Rule.rules.should_not contain Rule::Syntax
end
end

context "properties" do
Expand Down
4 changes: 2 additions & 2 deletions spec/ameba/config_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,14 @@ module Ameba
config = Config.load config_sample

it "updates enabled property" do
name = DummyRule.class_name
name = DummyRule.rule_name
config.update_rule name, enabled: false
rule = config.rules.find(&.name.== name).not_nil!
rule.enabled.should be_false
end

it "updates excluded property" do
name = DummyRule.class_name
name = DummyRule.rule_name
excluded = %w(spec/source.cr)
config.update_rule name, excluded: excluded
rule = config.rules.find(&.name.== name).not_nil!
Expand Down
2 changes: 1 addition & 1 deletion spec/ameba/formatter/todo_formatter_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ module Ameba
end

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

it "excludes source from this rule" do
Expand Down
2 changes: 1 addition & 1 deletion spec/ameba/rule/unneded_disable_directive_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ module Ameba::Rule

it "fails if there is disabled UnneededDisableDirective" do
s = Source.new %Q(
# ameba:disable #{UnneededDisableDirective.class_name}
# ameba:disable #{UnneededDisableDirective.rule_name}
a = 1
), "source.cr"
s.error UnneededDisableDirective.new, 3, 1, "Alarm!", :disabled
Expand Down
17 changes: 15 additions & 2 deletions spec/ameba/runner_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Ameba
config.formatter = formatter
config.files = files

config.update_rule ErrorRule.class_name, enabled: false
config.update_rule ErrorRule.rule_name, enabled: false

Runner.new(config)
end
Expand Down Expand Up @@ -59,7 +59,7 @@ module Ameba

Runner.new(rules, [source], formatter).run
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

it "does not run other rules" do
Expand All @@ -75,6 +75,19 @@ module Ameba
source.errors.size.should eq 1
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

describe "#success?" do
Expand Down
2 changes: 1 addition & 1 deletion src/ameba/config.cr
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ class Ameba::Config

def self.new(config = nil)
if (raw = config.try &.raw).is_a? Hash
yaml = raw[class_name]?.try &.to_yaml
yaml = raw[rule_name]?.try &.to_yaml
end
from_yaml yaml || "{}"
end
Expand Down
23 changes: 18 additions & 5 deletions src/ameba/rule/base.cr
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
module Ameba::Rule
# List of names of the special rules, which
# behave differently than usual rules.
SPECIAL = [
Syntax.class_name,
UnneededDisableDirective.class_name,
Syntax.rule_name,
UnneededDisableDirective.rule_name,
]

# Represents a base of all rules. In other words, all rules
Expand Down Expand Up @@ -60,7 +62,7 @@ module Ameba::Rule
# ```
#
def name
{{@type}}.class_name
{{@type}}.rule_name
end

# Checks whether the source is excluded from this rule.
Expand All @@ -78,7 +80,18 @@ module Ameba::Rule
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::", "")
end

Expand All @@ -95,6 +108,6 @@ module Ameba::Rule
# ```
#
def self.rules
Base.subclasses.reject! &.== Rule::Syntax
Base.subclasses
end
end
16 changes: 15 additions & 1 deletion src/ameba/runner.cr
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ module Ameba
# A syntax rule which always inspects a source first
@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`.
#
# ```
Expand All @@ -36,7 +39,11 @@ module Ameba
def initialize(config : Config)
@sources = load_sources(config)
@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

# :nodoc:
Expand Down Expand Up @@ -65,6 +72,7 @@ module Ameba
next if rule.excluded?(source)
rule.test(source)
end
check_unneeded_directives(source)
end

@formatter.source_finished source
Expand All @@ -87,6 +95,12 @@ module Ameba
@sources.all? &.valid?
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)
config.files.map do |wildcard|
wildcard += "/**/*.cr" if File.directory?(wildcard)
Expand Down

0 comments on commit eda5960

Please sign in to comment.