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.

Sign up for GitHub

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

Jump to bottom

Lint behaves weird when fixing a top-level structured fact reference #189

Closed
jay7x opened this issue Feb 5, 2024 · 6 comments · Fixed by #190
Closed

Lint behaves weird when fixing a top-level structured fact reference #189

jay7x opened this issue Feb 5, 2024 · 6 comments · Fixed by #190
Labels
bug Something isn't working community

Comments

@jay7x
Copy link

jay7x commented Feb 5, 2024

What Versions are you running?

OS Version: MacOS 14.2.1 (arm64)
VSCode Version: VSCodium Version: 1.85.2
Puppet Extension Version: v1.5.1 (from the open-vsx repo)
PDK Version: 3.0.1

What You Are Seeing?

Let's say I have the following example manifest with a (custom) top-level structured fact reference:

$foo = $::my_structured_fact['foo']

When I select "Format Document" from the context menu I got this:

$foo = $facts['my_structured_fact['foo']']

As you may see it's wrong syntax and it breaks the future file checks and formatting.

What is Expected?

$foo = $facts['my_structured_fact']['foo']
# -OR-
$foo = $facts.get('my_structured_fact.foo')

Though I'm not sure if it's easy to fix at all. So mostly leaving it here for your awareness.
@jordanbreen28 jordanbreen28 transferred this issue from puppetlabs/puppet-vscode Feb 5, 2024
@jordanbreen28
Copy link

found to be an issue with puppet-lint, issue can be replicated using the above manifest example and running lint with the -f flag.

@jordanbreen28 jordanbreen28 added the bug Something isn't working label Feb 5, 2024
@jordanbreen28 jordanbreen28 changed the title "Format Document" behaves weird when fixing a top-level structured fact reference Lint behaves weird when fixing a top-level structured fact reference Feb 5, 2024
jordanbreen28 added a commit that referenced this issue Feb 5, 2024
@jordanbreen28
(GH-189) - invalid fact correction for top scope structured fact
46b8c10 
Prior to this commit, if you had a top scope structured fact like this
in your manifest `$::my_structured_fact['foo']['test']`, lint would
return an invalid correction when passed the fix flag.

Now, we have altered the top_scope_facts plugin, to first check if it is
a structured fact, and if one is found using regex, apply different
logic to fix.
jordanbreen28 added a commit that referenced this issue Feb 5, 2024
@jordanbreen28
(GH-189) - invalid fact correction for top scope structured fact
eef5b0a 
Prior to this commit, if you had a top scope structured fact like this
in your manifest `$::my_structured_fact['foo']['test']`, lint would
return an invalid correction when passed the fix flag.

Now, we have altered the top_scope_facts plugin, to first check if it is
a structured fact, and if one is found using regex, apply different
logic to fix.
@jordanbreen28 jordanbreen28 mentioned this issue Feb 5, 2024
Merged
3 tasks
jordanbreen28 added a commit that referenced this issue Feb 5, 2024
@jordanbreen28
(GH-189) - invalid fact correction for top scope structured fact
531330b 
Prior to this commit, if you had a top scope structured fact like this
in your manifest `$::my_structured_fact['foo']['test']`, lint would
return an invalid correction when passed the fix flag.

Now, we have altered the top_scope_facts plugin, to first check if it is
a structured fact, and if one is found using regex, apply different
logic to fix.
jordanbreen28 added a commit that referenced this issue Feb 5, 2024
@jordanbreen28
(GH-189) - invalid fact correction for top scope structured fact
a7f1c44 
Prior to this commit, if you had a top scope structured fact like this
in your manifest `$::my_structured_fact['foo']['test']`, lint would
return an invalid correction when passed the fix flag.

Now, we have altered the top_scope_facts plugin, to first check if it is
a structured fact, and if one is found using regex, apply different
logic to fix.
jordanbreen28 added a commit that referenced this issue Feb 5, 2024
@jordanbreen28
(GH-189) - invalid fact correction for top scope structured fact
f73fd24 
Prior to this commit, if you had a top scope structured fact like this
in your manifest `$::my_structured_fact['foo']['test']`, lint would
return an invalid correction when passed the fix flag.

Now, we have altered the top_scope_facts plugin, to first check if it is
a structured fact, and if one is found using regex, apply different
logic to fix.
jordanbreen28 added a commit that referenced this issue Feb 5, 2024
@jordanbreen28
(GH-189) - invalid fact correction for top scope structured fact
d6b7625 
Prior to this commit, if you had a top scope structured fact like this
in your manifest `$::my_structured_fact['foo']['test']`, lint would
return an invalid correction when passed the fix flag.

Now, we have altered the top_scope_facts plugin, to first check if it is
a structured fact, and if one is found using regex, apply different
logic to fix.
jordanbreen28 added a commit that referenced this issue Feb 5, 2024
@jordanbreen28
(GH-189) - invalid fact correction for top scope structured fact
ec783e8 
Prior to this commit, if you had a top scope structured fact like this
in your manifest `$::my_structured_fact['foo']['test']`, lint would
return an invalid correction when passed the fix flag.

Now, we have altered the top_scope_facts plugin, to first check if it is
a structured fact, and if one is found using regex, apply different
logic to fix.
jordanbreen28 added a commit that referenced this issue Feb 6, 2024
@jordanbreen28
(GH-189) - invalid fact correction for top scope structured fact
0b22193 
Prior to this commit, if you had a top scope structured fact like this
in your manifest `$::my_structured_fact['foo']['test']`, lint would
return an invalid correction when passed the fix flag.

Now, we have altered the top_scope_facts plugin, to first check if it is
a structured fact, and if one is found using regex, apply different
logic to fix.
jordanbreen28 added a commit that referenced this issue Feb 6, 2024
@jordanbreen28
(GH-189) - invalid fact correction for top scope structured fact
2521343 
Prior to this commit, if you had a top scope structured fact like this
in your manifest `$::my_structured_fact['foo']['test']`, lint would
return an invalid correction when passed the fix flag.

Now, we have altered the top_scope_facts plugin, to first check if it is
a structured fact, and if one is found using regex, apply different
logic to fix.
jordanbreen28 added a commit that referenced this issue Feb 6, 2024
@jordanbreen28
(GH-189) - invalid fact correction for top scope structured fact
af161b7 
Prior to this commit, if you had a top scope structured fact like this
in your manifest `$::my_structured_fact['foo']['test']`, lint would
return an invalid correction when passed the fix flag.

Now, we have altered the top_scope_facts plugin, to first check if it is
a structured fact, and if one is found using regex, apply different
logic to fix.
jordanbreen28 added a commit that referenced this issue Feb 6, 2024
@jordanbreen28
(GH-189) - invalid fact correction for top scope structured fact
897f19a 
Prior to this commit, if you had a top scope structured fact like this
in your manifest `$::my_structured_fact['foo']['test']`, lint would
return an invalid correction when passed the fix flag.

Now, we have altered the top_scope_facts plugin, to first check if it is
a structured fact, and if one is found using regex, apply different
logic to fix.
jordanbreen28 added a commit that referenced this issue Feb 6, 2024
@jordanbreen28
(GH-189) - invalid fact correction for top scope structured fact
e4863b0 
Prior to this commit, if you had a top scope structured fact like this
in your manifest `$::my_structured_fact['foo']['test']`, lint would
return an invalid correction when passed the fix flag.

Now, we have altered the top_scope_facts plugin, to first check if it is
a structured fact, and if one is found using regex, apply different
logic to fix.
@jordanbreen28
Copy link

@jay7x I've raised #190 to fix this. All looks good on my end from my testing, would be great if you could verify this too :)

@jay7x
Copy link
Author

jay7x commented Feb 7, 2024

@jordanbreen28 Thank you! 🙏🏻 Not sure if I can test it quickly though..

@jordanbreen28
Copy link

jordanbreen28 commented Feb 7, 2024 •
edited

Not too worry! Always like to get the stamp of approval but im fairly certain it fixes the issue at hand anyway, we will get this merged in and released some time this week.
It might a week or two to roll this out to puppet-editor-services and subsequently puppet-vscode, but I'll be sure to update this thread when i do!
Thanks for raising this!

bastelfreak added a commit that referenced this issue Feb 9, 2024
@bastelfreak
Merge pull request #190 from puppetlabs/gh-189-fix_top_scope_facts_fix
a59e682 
(GH-189) - invalid fact correction for top scope structured fact
@jordanbreen28
Copy link

@jay7x this was shipped as part of v1.5.2 of puppet-vscode :)

@jay7x
Copy link
Author

jay7x commented Feb 15, 2024

@jordanbreen28 tyvm! Heading to try it later today!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working community
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

(GH-189) - invalid fact correction for top scope structured fact puppetlabs/puppet-lint
3 participants
@jay7x @ia-content @jordanbreen28