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

Wrong behaviour of variable_contains_dash #504

Closed
obourdon opened this issue Jul 7, 2016 · 13 comments · Fixed by #572
Closed

Wrong behaviour of variable_contains_dash #504

obourdon opened this issue Jul 7, 2016 · 13 comments · Fixed by #572
Milestone

Comments

@obourdon
Copy link

obourdon commented Jul 7, 2016

The following kind of code produce warnings without reason:

    Service["${plugin_zabbix::params::server_service}-init-stopped"] -> Cs_resource["p_${plugin_zabbix::params::server_service}"]

The code should check whether or not the variable is enclosed in angle-brackets or not

There are cases where it is necessary to "easily" concatenate strings without making the code unnecessariy more complex

@rnelson0
Copy link
Collaborator

rnelson0 commented Jul 7, 2016

@obourdon What version of puppet-lint are you using? I recently merged #500, which is related to variables with dashes, but it is only available via git rather than gem right now.

@torrancew
Copy link

@rnelson0 I have also been bitten by this. Will test this commit over the weekend and let you know.

@rnelson0
Copy link
Collaborator

rnelson0 commented Aug 2, 2016

@torrancew @obourdon Any updates? Thanks!

@rkhatibi
Copy link

I see what is going on.

my code
$var = "$var1-$var2"

puppet-lint --fix # v 2.0.0
$var = "${var1-}${var2}"

puppet-lint --fix # v 2.0.1
$var = "${var1}-${var2}"

So far so good. However the code on head now parses the fixed version of the code $var = "${var1}-${var2}" as WARNING: variable contains a dash on line 2

@rnelson0
Copy link
Collaborator

Thanks for the update. I'm not sure what the fix is yet, and focus is on the 2.0.1 release right now, but if a solution is found maybe we can fit it in. if not, next one

@bmfurtado
Copy link

I just started getting hit by exactly the described behaviour on an upgrade from v2.0.0 to v2.0.1.

Here is an easily reproducible example:

$ puppet-lint --version
puppet-lint 2.0.1
$ cat error.pp
$variable = 'hello'

notify { "${variable}-is-ok": }
$ puppet-lint error.pp
WARNING: variable contains a dash on line 3
$ puppet-lint --version
puppet-lint 2.0.0
$ cat error.pp
$variable = 'hello'

notify { "${variable}-is-ok": }
$ puppet-lint error.pp
$ echo $?
0

@jasonhancock
Copy link

I too am getting bitten by the same thing as @bmfurtado

@cyberious
Copy link

Yeah issue here as well, going to possibly revert to 2.0.0 as this is a heavy use case for use.

@rnelson0
Copy link
Collaborator

Alright, we will put this down for a quicker fix than 2.1.0. Traveling right now, though.

@rnelson0
Copy link
Collaborator

Just to be clear - 2.0.1 properly fixes the actual issue. It then throws a false positive saying there is still an issue. Correct?

@rnelson0 rnelson0 added this to the 2.0.2 milestone Aug 18, 2016
@rkhatibi
Copy link

Mostly correct, but slightly more complicated.

2.0.0 can't fix the problem correctly, but the check is correct.
2.0.1 can fix the problem correctly, but the check is incorrect.

In the short term I would prefer that we keep the check working rather than the fix.

@rnelson0
Copy link
Collaborator

2.0.2 was released which reverted the related change in 2.0.1 until it can be revisited properly. I am in Iowa, the place where cell signals go to die, so will not be able to do so myself for a while and hope this is a sufficient stopgap.

@steventwheeler
Copy link

Upgrading from v2.0.1 to v2.0.2 resolved this issue for me.

rdoproject pushed a commit to rdo-infra/puppet-dlrn that referenced this issue Apr 4, 2024
Patch Set 1:

Recheck

The puppet-lint error is caused by rodjek/puppet-lint#504, should be fixed by puppet-lint 2.0.2

Patch-set: 1
Reviewer: Gerrit User 113 <113@270e2033-b340-4cff-9539-693957ebf0e7>
Label: Workflow=0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants