Skip to content
New issue

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

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

Already on GitHub? # to your account

(GH-189) - invalid fact correction for top scope structured fact #190

Merged
merged 1 commit into from
Feb 9, 2024

Conversation

jordanbreen28
Copy link

@jordanbreen28 jordanbreen28 commented Feb 5, 2024

Summary

Prior to this PR, 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.

Additional Context

Unstructured top scope fact:

$foo = $::my_structured_fact

when fixed:

bundle exec puppet-lint ./manifest.pp -f
FIXED: top scope fact instead of facts hash on line 1 (check: top_scope_facts)
$foo = $facts['my_structured_fact']

top scope structured fact (& not on allowlist):

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

when fixed:

bundle exec puppet-lint ./manifest.pp -f
FIXED: top scope fact instead of facts hash on line 1 (check: top_scope_facts)
$foo = $facts['my_structured_fact']['foo']['test']

Related Issues (if any)

Fixes #189

Checklist

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

@jordanbreen28 jordanbreen28 force-pushed the gh-189-fix_top_scope_facts_fix branch from eef5b0a to 531330b Compare February 5, 2024 15:10
Copy link

codecov bot commented Feb 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6d5b00e) 93.13% compared to head (e4863b0) 93.15%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #190      +/-   ##
==========================================
+ Coverage   93.13%   93.15%   +0.01%     
==========================================
  Files          54       54              
  Lines        1763     1768       +5     
==========================================
+ Hits         1642     1647       +5     
  Misses        121      121              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jordanbreen28 jordanbreen28 force-pushed the gh-189-fix_top_scope_facts_fix branch 8 times, most recently from af161b7 to 897f19a Compare February 6, 2024 11:30
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 force-pushed the gh-189-fix_top_scope_facts_fix branch from 897f19a to e4863b0 Compare February 6, 2024 14:25
@jordanbreen28 jordanbreen28 marked this pull request as ready for review February 7, 2024 11:44
@jordanbreen28 jordanbreen28 requested review from bastelfreak and a team as code owners February 7, 2024 11:44
@bastelfreak bastelfreak merged commit a59e682 into main Feb 9, 2024
8 checks passed
@bastelfreak bastelfreak deleted the gh-189-fix_top_scope_facts_fix branch February 9, 2024 10:33
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lint behaves weird when fixing a top-level structured fact reference
2 participants