Add HtmlAttributes linter

Add linter that checks for the use of HTML-style attributes.

Change-Id: Ibbd8461c5c857cafc2d04ee1b977f7cbd8f05971
Reviewed-on: http://gerrit.causes.com/42567
Tested-by: jenkins <[email protected]>
Reviewed-by: Shane da Silva <[email protected]>

CHANGELOG.md

@@ -28,6 +28,8 @@
   of the `Haml::Parser::ParseNode` struct to make working with it easier
 * New lint `ObjectReferenceAttributes` checks for the use of the object
   reference syntax to set the class/id of an element
+* New lint `HtmlAttributes` checks for the use of the HTML-style attributes
+  syntax when defining attributes for an element
 
 ## 0.6.1
 

config/default.yml

@@ -21,6 +21,9 @@ linters:
   EmptyScript:
     enabled: true
 
+  HtmlAttributes:
+    enabled: true
+
   ImplicitDiv:
     enabled: true
 

lib/haml_lint/linter.rb

@@ -47,14 +47,6 @@ def contains_interpolation?(string)
       Haml::Util.contains_interpolation?(string)
     end
 
-    # Returns the portion of a string between two pairs of characters.
-    #
-    # @return [String, nil] the string matched within the balanced pair, or nil
-    #   if no balanced number of characters were found
-    def balance(string, start_char, close_char)
-      Haml::Util.balance(string, start_char, close_char)
-    end
-
     # Returns whether this tag node has inline script, e.g. is of the form
     # %tag= ...
     #

lib/haml_lint/linter/README.md

@@ -98,6 +98,28 @@ Don't write empty scripts.
 
 These serve no purpose and are usually left behind by mistake.
 
+## HtmlAttributes
+
+Don't use the
+[HTML-style attributes](http://haml.info/docs/yardoc/file.REFERENCE.html#htmlstyle_attributes_)
+syntax to define attributes for an element.
+
+**Bad**
+```haml
+%tag(lang=en)
+```
+
+**Good**
+```haml
+%tag{ lang: 'en' }
+```
+
+While the HTML-style attributes syntax can be terser, it introduces additional
+complexity to your templates as there are now two different ways to define
+attributes. Standardizing on when to use HTML-style versus hash-style adds
+greater cognitive load when writing templates. Using one style makes this
+easier.
+
 ## ImplicitDiv
 
 Avoid writing `%div` when it would otherwise be implicit.

lib/haml_lint/linter/html_attributes.rb

@@ -0,0 +1,14 @@
+module HamlLint
+  # Checks for the setting of attributes via HTML shorthand syntax on elements
+  # (e.g. `%tag(lang=en)`).
+  class Linter::HtmlAttributes < Linter
+    include LinterRegistry
+
+    def visit_tag(node)
+      return unless node.html_attributes?
+
+      add_lint(node, "Prefer the hash attributes syntax (%tag{ lang: 'en' }) over " \
+                     'HTML attributes syntax (%tag(lang=en))')
+    end
+  end
+end

lib/haml_lint/tree/tag_node.rb

@@ -75,15 +75,15 @@ def dynamic_attributes_source # rubocop:disable CyclomaticComplexity, MethodLeng
             case rest[0]
             when '{'
               break if hash_attributes
-              hash_attributes, rest = balance(rest, '{', '}')
+              hash_attributes, rest = Haml::Util.balance(rest, '{', '}')
               dynamic_attributes[:hash] = hash_attributes
             when '('
               break if html_attributes
-              html_attributes, rest = balance(rest, '(', ')')
+              html_attributes, rest = Haml::Util.balance(rest, '(', ')')
               dynamic_attributes[:html] = html_attributes
             when '['
               break if object_reference
-              object_reference, rest = balance(rest, '[', ']')
+              object_reference, rest = Haml::Util.balance(rest, '[', ']')
               dynamic_attributes[:object_ref] = object_reference
             else
               break

spec/haml_lint/linter/html_attributes_spec.rb

@@ -0,0 +1,29 @@
+require 'spec_helper'
+
+describe HamlLint::Linter::HtmlAttributes do
+  include_context 'linter'
+
+  context 'when a tag has no attributes' do
+    let(:haml) { '%tag' }
+
+    it { should_not report_lint }
+  end
+
+  context 'when a tag contains hash attributes' do
+    let(:haml) { "%tag{ lang: 'en' }" }
+
+    it { should_not report_lint }
+  end
+
+  context 'when a tag contains HTML attributes' do
+    let(:haml) { '%tag(lang=en)' }
+
+    it { should report_lint }
+  end
+
+  context 'when a tag contains HTML attributes and hash attributes' do
+    let(:haml) { '%tag(lang=en){ attr: value }' }
+
+    it { should report_lint }
+  end
+end