puppetlabs / puppet-lint

Check that your Puppet manifests conform to the style guide
https://puppetlabs.github.io/puppet-lint/
MIT License
18 stars 12 forks source link

Top scope variable are rewritten into fact references #195

Open brunoleon opened 4 months ago

brunoleon commented 4 months ago

Describe the Bug

The PR https://github.com/puppetlabs/puppet-lint/pull/190 introduces a bug that prevent reference to top scope variables by rewriting into a reference to a fact

$::example_var => save => $facts['example_var']

Expected Behavior

$::example_var => save => $::example_var (nothing to change)

Steps to Reproduce

Open VScode and enable Puppet Vscode Extension Write a manifests referencing a top scope variable. It is rewritten into a reference to a fact.

Environment

Since Puppet VSCode extension 1.5.0

kenyon commented 3 months ago

Seems correct to me. What top scope variables are you using?

brunoleon commented 3 months ago

The vscode extension assumes that all top scope variables are facts, which is not true.

You can have a variable defined in you "entrypoint" manifest that defines a variable to be used globally.

I'm not meaning this is the right thing to do (and I would even advise against...), however the current behavior prevents you from doing something that is possible in the language.

For example on one setup involving arm devices, I have the following piece of code because of repository naming;

//nodes.pp
$arch = $facts['os']['architecture'] ? {
  'aarch64' => 'arm64',
  'aarch'   => 'arm',
  default   => $facts['os']['architecture']
}

Current VSCode behavior prevents me from referring to this variable, it keeps turning it into a facts, which it is not.

Hope this makes sense

kenyon commented 3 months ago

In that case, shouldn't you be referring to it as $nodes::arch when used in other classes?

brunoleon commented 3 months ago

node.pp is an example entrypoint, may be be calling it site.pp would have been clearer but it is not a class

The documentation is here: https://www.puppet.com/docs/puppet/8/lang_scope.html#top-scope

Notably: _Because the top scope’s name is the empty string, $::my_variable refers to the top-scope value of $my_variable, even if $myvariable has a different value in local scope.

Which clearly states that we must be able to access top scope variables.

kenyon commented 3 months ago

OK I see. I don't use top scope variables from a site.pp like that, so thanks for elaborating.