puppet-lint icon indicating copy to clipboard operation
puppet-lint copied to clipboard

Unrecognized escape sequences don't raise warnings

Open glennsarti opened this issue 6 years ago • 3 comments

Given a manifest of:

# $number1 = 5000
# $number2 = '6000'
# $sum = $number1 + $number2
# notify{"Sum: $sum":}

$server = 'localhost'
notify{"Path: ${server}\foo":}

notify{"this string contains \l random esape": }

$ztring = "this ${server} string contains \l random esape"

#user { 'Bob'

Running puppet lint I get the following warnings:

C:\Source\puppet-editor-services [gh-211-lexer-warnings ≡ +0 ~4 -0 !]> be puppet-lint .\tmp\bah.pp
WARNING: double quoted string containing no variables on line 9

However when running puppet parser validate:

C:\Source\puppet-editor-services [gh-211-lexer-warnings ≡ +0 ~4 -0 !]> puppet parser validate tmp\bah.pp
Warning: Unrecognized escape sequence '\f' (file: C:/Source/puppet-editor-services/tmp/bah.pp, line: 7, column: 29)
Warning: Unrecognized escape sequence '\l' (file: C:/Source/puppet-editor-services/tmp/bah.pp, line: 9, column: 46)
Warning: Unrecognized escape sequence '\l' (file: C:/Source/puppet-editor-services/tmp/bah.pp, line: 11, column: 59)

Note this appears to be tested as part of https://github.com/rodjek/puppet-lint/blob/5bc211faf77afb66685b4ff13525652127cf7bbb/spec/puppet-lint/plugins/check_strings/double_quoted_strings_spec.rb#L114-L124 But it's not raising errors or warnings in Puppet Lint 2.4.2

Environment: OS - Windows 10 - 1903 Ruby - ruby 2.5.1p57 (2018-03-29 revision 63029) [x64-mingw32] Puppet Lint - puppet-lint 2.4.2

glennsarti avatar Dec 20 '19 03:12 glennsarti

Okay ... a quick shows the test is only accidentally working.

The "problem" it's getting is;

{:message=>"double quoted string containing no variables", :line=>1, :column=>12, :token=><Token :STRING (this string contains l random esape) @1:12>, :kind=>:warning, :check=>:double_quoted_strings, :fullpath=>"C:/Source/puppet-lint", :path=>"", :filename=>""}

If you change the test string to $ztring = "this ${string} contains \l random esape" so it doesn't trigger the "no variables" rule you get the following:

double_quoted_strings
  with fix disabled
    double quoted string with random escape should be rejected
      should only detect a single problem (FAILED - 1)
      should create a warning (FAILED - 2)

Failures:

  1) double_quoted_strings with fix disabled double quoted string with random escape should be rejected should only detect a single problem
     Failure/Error: expect(problems).to have(1).problem
       expected 1 problem, got 0
     # ./spec/puppet-lint/plugins/check_strings/double_quoted_strings_spec.rb:118:in `block (4 levels) in <top (required)>'

  2) double_quoted_strings with fix disabled double quoted string with random escape should be rejected should create a warning
     Failure/Error: expect(problems).to contain_warning(msg).on_line(1).in_column(12)
       expected that the check would create a problem but it did not
     # ./spec/puppet-lint/plugins/check_strings/double_quoted_strings_spec.rb:123:in `block (4 levels) in <top (required)>'

glennsarti avatar Dec 20 '19 03:12 glennsarti

I believe this statement in the style guide would warrant puppet-lint raising an error for it

Do not rely on unrecognized escaped characters as a method for including the backslash and the character following it.

glennsarti avatar Dec 20 '19 03:12 glennsarti

After taking a look at the tests and the implementation I read this issue as a feature request. The current implementation doesn't try to warn on unrecognized escape sequences and the mentioned test was explicitly written to warn about double quoting the string with a random escape sequence -- not to warn about the escaping itself.

That being said, I'd also like to get a warning from the linter about unrecognized escape sequences. Probably it could be done in a plugin as well.

usev6 avatar Dec 26 '19 20:12 usev6