Skip to content

Commit

Permalink
Add PrivateNamingConvention linter
Browse files Browse the repository at this point in the history
Although the `NameFormat` linter has an option to allow for leading
underscores, it does not enforce the convention. It would be really
great to either have a linter that does this.

What I mean by enforcing privacy for the leading underscore convention
is, for variables, functions, and mixins that start with a leading
underscore, we should verify that:

- any definitions are used within the same file
- any uses are defined within the same file
- any uses are preceded by their definitions

I considered adding this to `NameFormat`, but it seems specialized
enough that I decided to put it in its own linter. I left the prefix
configurable but defaulting to `_`, which is likely what most people
would use anyway.

Fixes #721
  • Loading branch information
lencioni committed Feb 18, 2016
1 parent 1f1b66e commit 896bbe0
Show file tree
Hide file tree
Showing 5 changed files with 404 additions and 0 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
@@ -1,5 +1,11 @@
# SCSS-Lint Changelog

## master (unreleased)

* Add `PrivateNamingConvention` linter which enforces that functions, mixins,
and variables that follow the private naming convention (default to
underscore-prefixed) are defined and used within the same file

## 0.45.0

* Add `outline-{color,style,width}` properties to `recess` sort order list
Expand Down
4 changes: 4 additions & 0 deletions config/default.yml
Expand Up @@ -116,6 +116,10 @@ linters:
PlaceholderInExtend:
enabled: true

PrivateNamingConvention:
enabled: false
prefix: _

PropertyCount:
enabled: false
include_nested: false
Expand Down
47 changes: 47 additions & 0 deletions lib/scss_lint/linter/README.md
Expand Up @@ -31,6 +31,7 @@ Below is a list of linters supported by `scss-lint`, ordered alphabetically.
* [NameFormat](#nameformat)
* [NestingDepth](#nestingdepth)
* [PlaceholderInExtend](#placeholderinextend)
* [PrivateNamingConvention](#privatenamingconvention)
* [PropertyCount](#propertycount)
* [PropertySortOrder](#propertysortorder)
* [PropertySpelling](#propertyspelling)
Expand Down Expand Up @@ -857,6 +858,52 @@ See [Mastering Sass extends and placeholders](http://8gramgorilla.com/mastering-
If you want to prevent the use of the `@extend` directive entirely, see the
[ExtendDirective](#extenddirective) linter.

## PrivateNamingConvention

**Disabled by default**

Enforces that functions, mixins, and variables that follow the private naming
convention (default to underscore-prefixed, e.g. `$_foo`) are defined and used
within the same file.

**Bad**
```scss
$_foo: #f00;

p {
color: #00f;
}
```

**Bad**
```scss
p {
color: $_foo;
}
```

**Bad**
```scss
p {
color: $_foo;
}

$_foo: #f00;
```

**Good**
```scss
$_foo: #f00;

p {
color: $_foo;
}
```

Configuration Option | Description
---------------------|----------------------------------------------
`prefix` | Prefix used to denote "private" (default `_`)

## PropertyCount

**Disabled by default**
Expand Down
86 changes: 86 additions & 0 deletions lib/scss_lint/linter/private_naming_convention.rb
@@ -0,0 +1,86 @@
module SCSSLint
# Verifies that variables, functions, and mixins that follow the private naming convention are
# defined and used within the same file.
class Linter::PrivateNamingConvention < Linter
include LinterRegistry

def visit_root(node)
# Register all top-level function, mixin, and variable definitions.
node.children.each_with_object([]) do |child_node|
if child_node.is_a?(Sass::Tree::FunctionNode)
register_node child_node, 'function'
elsif child_node.is_a?(Sass::Tree::MixinDefNode)
register_node child_node, 'mixin'
elsif child_node.is_a?(Sass::Tree::VariableNode)
register_node child_node, 'variable'
else
yield
end
end

# After we have visited everything, we want to see if any private things
# were defined but not used.
after_visit_all
end

def visit_script_funcall(node)
check_privacy(node, 'function')
yield # Continue linting any arguments of this function call
end

def visit_mixin(node)
check_privacy(node, 'mixin')
yield # Continue into content block of this mixin's block
end

def visit_script_variable(node)
check_privacy(node, 'variable')
end

private

def register_node(node, node_type, node_text = node.name)
return unless private?(node)

@private_definitions ||= {}
@private_definitions[node_type] ||= {}

@private_definitions[node_type][node_text] = {
node: node,
times_used: 0,
}
end

def check_privacy(node, node_type, node_text = node.name)
return unless private?(node)

if @private_definitions &&
@private_definitions[node_type] &&
@private_definitions[node_type][node_text]
@private_definitions[node_type][node_text][:times_used] += 1
return
end

add_lint(
node, "Private #{node_type} #{node_text} must be defined in the same file it is used")
end

def private?(node)
node.name.start_with?(config['prefix'])
end

def after_visit_all
return unless @private_definitions

@private_definitions.each do |node_type, nodes|
nodes.each do |node_text, node_info|
next if node_info[:times_used] > 0
add_lint(
node_info[:node],
"Private #{node_type} #{node_text} must be used in the same file it is defined"
)
end
end
end
end
end

0 comments on commit 896bbe0

Please sign in to comment.