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

Add Error type to the lexer tokens (and sort them) #216

Merged
merged 2 commits into from
Sep 13, 2024

Conversation

jay7x
Copy link

@jay7x jay7x commented Aug 22, 2024

Summary

Error type wasn't known to the lexer. That makes puppet-lint to fire unquoted_string_in_{selector|case} on this type check.

Additional Context

Consider this code:

$x ? {
  Error => false,
  default => true,
}

Puppet-lint will raise a warning on the line containing the Error.

Related Issues (if any)

Checklist

  • 🟢 Spec tests.
  • 🟢 Acceptance tests.
  • Manually verified.

@jay7x jay7x requested review from bastelfreak and a team as code owners August 22, 2024 04:16
@jay7x
Copy link
Author

jay7x commented Aug 22, 2024

JFYI, the change is untested.. I'm unable to test it right now.

@@ -123,7 +123,7 @@ def heredoc_queue
[:WHITESPACE, %r{\A(#{WHITESPACE_RE}+)}],
# FIXME: Future breaking change, the following :TYPE tokens conflict with
# the :TYPE keyword token.
[:TYPE, %r{\A(Integer|Float|Boolean|Regexp|String|Array|Hash|Resource|Class|Collection|Scalar|Numeric|CatalogEntry|Data|Tuple|Struct|Optional|NotUndef|Variant|Enum|Pattern|Any|Callable|Type|Runtime|Undef|Default|Sensitive)\b}], # rubocop:disable Layout/LineLength
[:TYPE, %r{\A(Any|Array|Boolean|Callable|CatalogEntry|Class|Collection|Data|Default|Enum|Error|Float|Hash|Integer|NotUndef|Numeric|Optional|Pattern|Regexp|Resource|Runtime|Scalar|Sensitive|String|Struct|Tuple|Type|Undef|Variant)\b}], # rubocop:disable Layout/LineLength
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the sorting by name, but could you please do that in a second commit? That makes it a bit easier to read the diff.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do in a few days

Copy link
Author

@jay7x jay7x Aug 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I split the change to 2 commits. Please enjoy :-D

@david22swan david22swan added the enhancement New feature or request label Aug 30, 2024
Copy link
Member

@david22swan david22swan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, if you could add a small unit test for this we would be happy to go ahead and merge.
https://github.com/puppetlabs/puppet-lint/blob/main/spec/unit/puppet-lint/lexer_spec.rb

@jay7x
Copy link
Author

jay7x commented Aug 30, 2024

I added some naive test. Not sure if it's good enough.

Copy link

@jordanbreen28 jordanbreen28 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@jordanbreen28 jordanbreen28 merged commit c2c9e8a into puppetlabs:main Sep 13, 2024
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants