Add PrivateNamingConvention linter

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

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

config/default.yml

@@ -116,6 +116,10 @@ linters:
   PlaceholderInExtend:
     enabled: true
 
+  PrivateNamingConvention:
+    enabled: false
+    prefix: _
+
   PropertyCount:
     enabled: false
     include_nested: false

lib/scss_lint/linter.rb

@@ -39,6 +39,7 @@ def run(engine, config)
       @engine = engine
       @comment_processor = ControlCommentProcessor.new(self)
       visit(engine.tree)
+      after_visit_all
       @lints = @comment_processor.filter_lints(@lints)
     end
 
@@ -50,6 +51,12 @@ def name
 
   protected
 
+    # Called after visiting all of the nodes
+    def after_visit_all
+      # This is a no-op by default. If you are writing a linter that needs to do some processing
+      # after visiting all nodes, you can define this method in your linter.
+    end
+
     # Helper for creating lint from a parse tree node
     #
     # @param node_or_line_or_location [Sass::Script::Tree::Node, Fixnum, SCSSLint::Location]

lib/scss_lint/linter/README.md

@@ -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)
@@ -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**

lib/scss_lint/linter/private_naming_convention.rb

@@ -0,0 +1,82 @@
+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_function(node)
+      register_node(node, 'function')
+      yield # Continue into content block of this function definition
+    end
+
+    def visit_script_funcall(node)
+      check_privacy(node, 'function')
+      yield # Continue linting any arguments of this function call
+    end
+
+    def visit_mixindef(node)
+      register_node(node, 'mixin')
+      yield # Continue into content block of this mixin definition
+    end
+
+    def visit_mixin(node)
+      check_privacy(node, 'mixin')
+      yield # Continue into content block of this mixin's block
+    end
+
+    def visit_variable(node)
+      register_node(node, 'variable')
+      yield # Continue into expression tree for this variable definition
+    end
+
+    def visit_script_variable(node)
+      check_privacy(node, 'variable')
+    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
+
+  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
+  end
+end

spec/scss_lint/linter/private_naming_convention_spec.rb

@@ -0,0 +1,177 @@
+require 'spec_helper'
+
+describe SCSSLint::Linter::PrivateNamingConvention do
+  context 'when a private variable' do
+    context 'is not used in the same file it is defined' do
+      let(:scss) { <<-SCSS }
+        $_foo: red;
+      SCSS
+
+      it { should report_lint line: 1 }
+    end
+
+    context 'is used but not defined in the same file' do
+      let(:scss) { <<-SCSS }
+        p {
+          color: $_foo;
+        }
+      SCSS
+
+      it { should report_lint line: 2 }
+    end
+
+    context 'is used but has not been defined quite yet' do
+      let(:scss) { <<-SCSS }
+        p {
+          color: $_foo;
+        }
+
+        $_foo: red;
+      SCSS
+
+      it { should report_lint line: 2 }
+    end
+
+    context 'is defined and used in the same file' do
+      let(:scss) { <<-SCSS }
+        $_foo: red;
+
+        p {
+          color: $_foo;
+        }
+      SCSS
+
+      it { should_not report_lint }
+    end
+  end
+
+  context 'when a public variable is used but not defined' do
+    let(:scss) { <<-SCSS }
+      p {
+        color: $foo;
+      }
+    SCSS
+
+    it { should_not report_lint }
+  end
+
+  context 'when a private mixin' do
+    context 'is not used in the same file it is defined' do
+      let(:scss) { <<-SCSS }
+        @mixin _foo {
+          color: red;
+        }
+      SCSS
+
+      it { should report_lint line: 1 }
+    end
+
+    context 'is used but not defined in the same file' do
+      let(:scss) { <<-SCSS }
+        p {
+          @include _foo;
+        }
+      SCSS
+
+      it { should report_lint line: 2 }
+    end
+
+    context 'is used but has not been defined quite yet' do
+      let(:scss) { <<-SCSS }
+        p {
+          @include _foo;
+        }
+
+        @mixin _foo {
+          color: red;
+        }
+      SCSS
+
+      it { should report_lint line: 2 }
+    end
+
+    context 'is defined and used in the same file' do
+      let(:scss) { <<-SCSS }
+        @mixin _foo {
+          color: red;
+        }
+
+        p {
+          @include _foo;
+        }
+      SCSS
+
+      it { should_not report_lint }
+    end
+  end
+
+  context 'when a public mixin is used but not defined' do
+    let(:scss) { <<-SCSS }
+      p {
+        @include foo;
+      }
+    SCSS
+
+    it { should_not report_lint }
+  end
+
+  describe 'when a private function' do
+    context 'is not used in the same file it is defined' do
+      let(:scss) { <<-SCSS }
+        @function _foo() {
+          @return red;
+        }
+      SCSS
+
+      it { should report_lint line: 1 }
+    end
+
+    context 'is used but not defined in the same file' do
+      let(:scss) { <<-SCSS }
+        p {
+          color: _foo();
+        }
+      SCSS
+
+      it { should report_lint line: 2 }
+    end
+
+    context 'is used but has not been defined quite yet' do
+      let(:scss) { <<-SCSS }
+        p {
+          color: _foo();
+        }
+
+        @function _foo() {
+          @return red;
+        }
+      SCSS
+
+      it { should report_lint line: 2 }
+    end
+
+    context 'is defined and used in the same file' do
+      let(:scss) { <<-SCSS }
+        @function _foo() {
+          @return red;
+        }
+
+        p {
+          color: _foo();
+        }
+      SCSS
+
+      it { should_not report_lint }
+    end
+  end
+
+  context 'when a public function is used but not defined' do
+    let(:scss) { <<-SCSS }
+      p {
+        color: foo();
+      }
+    SCSS
+
+    it { should_not report_lint }
+  end
+end