Merge pull request #621 from rodjek/issue-416

Correctly handle strings-with-variables as hash keys in arrow_alignment check

lib/puppet-lint/lexer.rb

@@ -309,6 +309,16 @@ def new_token(type, value, length, opts = {})
       end
 
       @column += length
+
+      # If creating a :VARIABLE token inside a double quoted string, add 3 to
+      # the column state in order to account for the ${} characters when
+      # rendering out to manifest.
+      if token.type == :VARIABLE
+        if !token.prev_code_token.nil? && [:DQPRE, :DQMID].include?(token.prev_code_token.type)
+          @column += 3
+        end
+      end
+
       if type == :NEWLINE
         @line_no += 1
         @column = 1

lib/puppet-lint/plugins/check_whitespace.rb

@@ -146,11 +146,27 @@ def check
           (level_tokens[indent_depth_idx] ||= []) << token
           param_token = token.prev_code_token
 
+          if param_token.type == :DQPOST
+            param_length = 0
+            iter_token = param_token
+            while iter_token.type != :DQPRE do
+              param_length += iter_token.to_manifest.length
+              iter_token = iter_token.prev_token
+            end
+            param_length += iter_token.to_manifest.length
+          else
+            param_length = param_token.to_manifest.length
+          end
+
           if param_column[indent_depth_idx].nil?
-            param_column[indent_depth_idx] = param_token.column
+            if param_token.type == :DQPOST
+              param_column[indent_depth_idx] = iter_token.column
+            else
+              param_column[indent_depth_idx] = param_token.column
+            end
           end
 
-          indent_length = param_column[indent_depth_idx] + param_token.to_manifest.length + 1
+          indent_length = param_column[indent_depth_idx] + param_length + 1
 
           if indent_depth[indent_depth_idx] < indent_length
             indent_depth[indent_depth_idx] = indent_length
@@ -185,7 +201,19 @@ def check
   end
 
   def fix(problem)
-    new_ws_len = (problem[:indent_depth] - (problem[:newline_indent] + problem[:token].prev_code_token.to_manifest.length + 1))
+    param_token = problem[:token].prev_code_token
+    if param_token.type == :DQPOST
+      param_length = 0
+      iter_token = param_token
+      while iter_token.type != :DQPRE do
+        param_length += iter_token.to_manifest.length
+        iter_token = iter_token.prev_token
+      end
+      param_length += iter_token.to_manifest.length
+    else
+      param_length = param_token.to_manifest.length
+    end
+    new_ws_len = (problem[:indent_depth] - (problem[:newline_indent] + param_length + 1))
     new_ws = ' ' * new_ws_len
     if problem[:newline]
       index = tokens.index(problem[:token].prev_code_token.prev_token)

spec/puppet-lint/lexer_spec.rb

@@ -516,6 +516,12 @@
       expect(tokens[2].line).to eq(1)
       expect(tokens[2].column).to eq(6)
     end
+
+    it 'should calculate the column number correctly after an enclosed variable' do
+      token = @lexer.tokenise('  "${foo}" =>').last
+      expect(token.type).to eq(:FARROW)
+      expect(token.column).to eq(12)
+    end
   end
 
   [

spec/puppet-lint/plugins/check_whitespace/arrow_alignment_spec.rb

@@ -365,6 +365,43 @@ class { 'lvs::base':
         expect(problems).to contain_warning(sprintf(msg,18,17)).on_line(5).in_column(17)
       end
     end
+
+    context 'hash with strings containing variables as keys properly aligned' do
+      let(:code) { '
+        foo { foo:
+          param => {
+            a         => 1
+            "${aoeu}" => 2,
+            b         => 3,
+          },
+        }
+      ' }
+
+      it 'should not detect any problems' do
+        expect(problems).to have(0).problems
+      end
+    end
+
+    context 'hash with strings containing variables as keys incorrectly aligned' do
+      let(:code) { '
+        foo { foo:
+          param => {
+            a => 1
+            "${aoeu}" => 2,
+            b     => 3,
+          },
+        }
+      ' }
+
+      it 'should detect 2 problems' do
+        expect(problems).to have(2).problems
+      end
+
+      it 'should create 2 warnings' do
+        expect(problems).to contain_warning(sprintf(msg,23,15)).on_line(4).in_column(15)
+        expect(problems).to contain_warning(sprintf(msg,23,19)).on_line(6).in_column(19)
+      end
+    end
   end
 
   context 'with fix enabled' do
@@ -603,5 +640,40 @@ class { 'lvs::base':
         expect(manifest).to eq(fixed)
       end
     end
+
+    context 'hash with strings containing variables as keys incorrectly aligned' do
+      let(:code) { '
+        foo { foo:
+          param => {
+            a => 1
+            "${aoeu}" => 2,
+            b     => 3,
+          },
+        }
+      ' }
+      let(:fixed) { '
+        foo { foo:
+          param => {
+            a         => 1
+            "${aoeu}" => 2,
+            b         => 3,
+          },
+        }
+      ' }
+
+
+      it 'should detect 2 problems' do
+        expect(problems).to have(2).problems
+      end
+
+      it 'should fix 2 problems' do
+        expect(problems).to contain_fixed(sprintf(msg,23,15)).on_line(4).in_column(15)
+        expect(problems).to contain_fixed(sprintf(msg,23,19)).on_line(6).in_column(19)
+      end
+
+      it 'should align the hash rockets' do
+        expect(manifest).to eq(fixed)
+      end
+    end
   end
 end