Ignore data types where trailing commas are illegal #25
Conversation
chelnak
force-pushed
the
GH-16-ignore_enum_arrays
branch
2 times, most recently
from
8b9400c
to
7b48fd1
Compare
|
Doesn't look like these test failures were caused by me! |
|
I think #26 should fix that. |
|
@chelnak tha ks for the PR. can you please rebase and add a unit test with your examples? |
|
Yeah sure thing! |
Prior to this commit the plugin would evaluate a data types value array as a
standard array type. This would cause the line to be flagged as invalid.
When running with `--fix`, the plugin would attempt to fix this condition
by appending a comma token to the final element of the array.i
According to puppet-syntax this is invalid.
For example:
Vaid:
```
Enum[
'enforcing',
'permissive',
'disabled'
] $selinux_setting = 'permissive',
```
Invalid:
```
Enum[
'enforcing',
'permissive',
'disabled',
] $selinux_setting = 'permissive',
```
Running `bundle exec rake syntax` post fix returns the following
error:
```
Could not parse for environment *root*: Syntax error at ']' (file: ./test.pp, line: 7, column: 1)
```
The above is not true for normal array declarations. Puppet syntax will
happily parse a normal array with a trailing comma.
Valid auto-fix from the plugin
```
$test = [
'one',
'two',
'three',
]
```
This commit fixes the above by adding functionality to check whether the current
`array_index` is actually an array of data type values.
If the condition is true we skip the current iteration so that it will
not be linted.
This commit updates the existing spec tests with examples that demonstrate how the linter would ignore data type values.
Codecov ReportBase: 100.00% // Head: 100.00% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## master #25 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 60 61 +1
=========================================
+ Hits 60 61 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
chelnak
force-pushed
the
GH-16-ignore_enum_arrays
branch
from
99852ea
to
e425048
Compare
|
Tests added an rebased. From testing on my side, I'm pretty sure that this fixes the original issue. However, there might be cases that I haven't thought about or some further hardening that we can do. Also I was purposely explicit with the extra next statement as I thought it might enhance readability somewhat.. More than happy to refactor this though. Just let me know what you think 👍 |
|
I've also opened puppetlabs/puppet-lint#48 Even though it might take a while for everything to converge it should mean that we can remove a bit of duplication here in the future. |
|
Thanks for the merge! Is there anything that I can do to help get this released? |
ekohl
changed the title
|
I wanted to add you as a reviewer on #28 but I noticed you're not in the VP org so I've invited you. |
|
Thank you. I've joined, reviewed and approved. |
Prior to this PR the plugin would evaluate a data types value array as a standard array type. This would cause the line to be flagged as invalid.
When running with
--fix, the plugin would attempt to fix this condition by appending a comma token to the final element of the array.iAccording to puppet-syntax this is invalid.
For example:
Vaid:
Invalid:
Running
bundle exec rake syntaxpost fix returns the followingerror:
The above is not true for normal array declarations. Puppet syntax will
happily parse a normal array with a trailing comma.
Valid auto-fix from the plugin
This PR fixes the above by adding functionality to check whether the current
array_indexis actually an array of data type values.If the condition is true we skip the current iteration so that it will not be linted.