Skip to content

Commit

Permalink
Lookup ignore files using the linter's name (houndci#1118)
Browse files Browse the repository at this point in the history
* Lookup ignore files using the linter's name

There was a bug where `JsIgnore` looked up the ignore file only for the
`javascript` linter. It now takes in a linter name and uses that as the
key to look up in `.hound.yml`.

* Set MultilineMethodCallIndentation Ruby rule to be indented
  • Loading branch information
Greg Lazarev committed Apr 29, 2016
1 parent f13edd6 commit fa8c286
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 19 deletions.
8 changes: 8 additions & 0 deletions .ruby-style.yml
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,14 @@ Style/MethodName:
SupportedStyles:
- snake_case
- camelCase
Style/MultilineMethodCallIndentation:
Description: Checks indentation of method calls with the dot operator
that span more than one line.
Enabled: true
EnforcedStyle: indented
SupportedStyles:
- aligned
- indented
Style/MultilineOperationIndentation:
Description: Checks indentation of binary operations that span more than one line.
Enabled: true
Expand Down
2 changes: 1 addition & 1 deletion app/models/linter/eslint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def file_included?(commit_file)
private

def jsignore
@jsignore ||= JsIgnore.new(hound_config, IGNORE_FILENAME)
@jsignore ||= JsIgnore.new(name, hound_config, IGNORE_FILENAME)
end
end
end
2 changes: 1 addition & 1 deletion app/models/linter/jshint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def file_included?(commit_file)
private

def jsignore
@jsignore ||= JsIgnore.new(hound_config, IGNORE_FILENAME)
@jsignore ||= JsIgnore.new(name, hound_config, IGNORE_FILENAME)
end
end
end
11 changes: 3 additions & 8 deletions lib/js_ignore.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
class JsIgnore
DEFAULT_EXCLUDED_PATHS = %w(vendor/*).freeze

def initialize(hound_config, ignore_filename)
@hound_config = hound_config
@ignore_filename = ignore_filename
end
attr_private_initialize :linter_name, :hound_config, :default_filename

def file_included?(commit_file)
excluded_paths.none? do |pattern|
Expand All @@ -14,8 +11,6 @@ def file_included?(commit_file)

private

attr_reader :hound_config, :ignore_filename

def excluded_paths
ignored_paths.presence || DEFAULT_EXCLUDED_PATHS
end
Expand All @@ -31,7 +26,7 @@ def ignored_paths
def ignore_filename
@ignore_filename ||= hound_config.
content.
fetch("javascript", {}).
fetch("ignore_file", ignore_filename)
fetch(linter_name, {}).
fetch("ignore_file", default_filename)
end
end
43 changes: 34 additions & 9 deletions spec/lib/js_ignore_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,31 @@
describe "#file_included?" do
context "file is in excluded file list" do
it "returns false" do
jsignore = build_js_ignore(["foo.js"])
commit_file = double("CommitFile", filename: "foo.js")
jsignore = build_js_ignore("javascript", "foo/*")
commit_file = double("CommitFile", filename: "foo/bar.js")

expect(jsignore.file_included?(commit_file)).to eq false
end

context "with a different linter" do
it "returns false" do
jsignore = build_js_ignore("eslint", "foo/*")
commit_file = instance_double("CommitFile", filename: "foo/bar.js")

expect(jsignore.file_included?(commit_file)).to be false
end
end
end

context "file is not excluded" do
it "returns true" do
jsignore = build_js_ignore(["foo.js"])
commit_file = double("CommitFile", filename: "bar.js")
jsignore = build_js_ignore("javascript", "foo/*")
commit_file = double("CommitFile", filename: "foo.js")

expect(jsignore.file_included?(commit_file)).to eq true
end

it "matches a glob pattern" do
jsignore = build_js_ignore(["app/assets/javascripts/*.js", "vendor/*"])
commit_file1 = double(
"CommitFile",
filename: "app/assets/javascripts/bar.js",
Expand All @@ -30,17 +38,34 @@
"CommitFile",
filename: "vendor/assets/javascripts/foo.js",
)
ignore_file_content = <<~TEXT
app/assets/javascripts/*.js\n
vendor/*
TEXT
jsignore = build_js_ignore("javascript", ignore_file_content)

expect(jsignore.file_included?(commit_file1)).to be false
expect(jsignore.file_included?(commit_file2)).to be false
end
end

def build_js_ignore(paths)
jsignore = JsIgnore.new({}, ".jsignore")
allow(jsignore).to receive(:ignored_paths).and_return(paths)
def build_js_ignore(linter, content)
hound_config = build_hound_config(linter, ".jsignore", content)

JsIgnore.new("javascript", hound_config, ".jsignore")
end

def build_hound_config(linter, ignore_filename, content)
config_content = {
linter => {
"ignore_file" => ignore_filename,
},
}
commit = instance_double("Commit")
allow(commit).to receive(:file_content).with(ignore_filename).
and_return(content)

jsignore
instance_double("HoundConfig", content: config_content, commit: commit)
end
end
end

0 comments on commit fa8c286

Please sign in to comment.