Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unrecognized escape sequences don't raise warnings #907

Open
glennsarti opened this issue Dec 20, 2019 · 3 comments
Open

Unrecognized escape sequences don't raise warnings #907

glennsarti opened this issue Dec 20, 2019 · 3 comments

Comments

@glennsarti
Copy link

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

context 'double quoted string with random escape should be rejected' do
let(:code) { %( $ztring = "this string contains \l random esape" ) }
it 'should only detect a single problem' do
expect(problems).to have(1).problem
end
it 'should create a warning' do
expect(problems).to contain_warning(msg).on_line(1).in_column(12)
end
end
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
Copy link
Author

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
Copy link
Author

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.

@usev6
Copy link
Contributor

usev6 commented Dec 26, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants