Skip to content

Commit

Permalink
Handle files that begin with comments in PrivateNamingConvention
Browse files Browse the repository at this point in the history
After trying out this new rule, I noticed a couple of bugs. Both are
related, and one caused a crash. They both happen when the file begins
with a comment.

For some reason, if the file starts with a comment, the engine seems to
visit the function usage twice, once before it visits the function
definition, and once after. Since we are relying on the order that these
things are visited here, this causes a lint to happen.

But even worse, when visiting a variable within a for loop, the node has
no source range, which causes a crash. This only seems to happen when
the file begins with a comment.

I fixed these by making sure that things have a source range before we
determine their position, and by also checking the top level even though
we really shouldn't need to.
  • Loading branch information
lencioni committed Feb 23, 2016
1 parent 5c1fd8e commit 55047bb
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 4 deletions.
6 changes: 2 additions & 4 deletions lib/scss_lint/linter/private_naming_convention.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,7 @@ def node_defined_earlier_in_branch?(node_to_look_in, looking_for)
return true # We found a match, so we are done
end

# We are at the top of the branch and don't want to check the root branch,
# since that is handled elsewhere, which means that we did not find a
# match.
return false unless node_to_look_in.node_parent.node_parent
return false unless node_to_look_in.node_parent

# We did not find a match yet, and haven't reached the top of the branch,
# so recurse.
Expand All @@ -122,6 +119,7 @@ def private?(node)
end

def before?(node, before_location)
return true unless node.source_range
location = location_from_range(node.source_range)
return true if location.line < before_location.line
if location.line == before_location.line &&
Expand Down
32 changes: 32 additions & 0 deletions spec/scss_lint/linter/private_naming_convention_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,38 @@
it { should_not report_lint }
end

context 'is defined and used in a for loop when the file begins with a comment' do
let(:scss) { <<-SCSS }
// A comment
@function _foo() {
@return red;
}
@for $i from 0 through 1 {
.foo {
color: _foo();
}
}
SCSS

it { should_not report_lint }
end

context 'is defined and used in the same file when the file begins with a comment' do
let(:scss) { <<-SCSS }
// A comment
@function _foo() {
@return red;
}
.foo {
color: _foo();
}
SCSS

it { should_not report_lint }
end

context 'is defined within a selector and not used' do
let(:scss) { <<-SCSS }
p {
Expand Down

0 comments on commit 55047bb

Please sign in to comment.