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

Add PowerShell formatter linter #2176

Merged
merged 10 commits into from
Dec 26, 2022

Conversation

bdovaz
Copy link
Collaborator

@bdovaz bdovaz commented Dec 23, 2022

@nvuillam I put it in draft because I have the doubt of how to create a test that fails if this command never fails... I pass it an input that is not even a valid powershell code and it works, that is, it does nothing and returns the same input as output....

https://learn.microsoft.com/en-us/powershell/module/psscriptanalyzer/invoke-formatter?view=ps-modules

Formatter code that does not handle any errors:

https://github.com/PowerShell/PSScriptAnalyzer/blob/1.21.0/Engine/Formatter.cs

Unlike ScriptAnalyzer.cs which does handle errors (used by original powershell linter):

https://github.com/PowerShell/PSScriptAnalyzer/blob/1.21.0/Engine/ScriptAnalyzer.cs

@codecov-commenter
Copy link

codecov-commenter commented Dec 23, 2022

Codecov Report

Merging #2176 (84db0ea) into main (c9e2d37) will increase coverage by 0.45%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main    #2176      +/-   ##
==========================================
+ Coverage   82.36%   82.81%   +0.45%     
==========================================
  Files         167      167              
  Lines        4439     4445       +6     
==========================================
+ Hits         3656     3681      +25     
+ Misses        783      764      -19     
Impacted Files Coverage Δ
megalinter/linters/PowershellLinter.py 81.48% <50.00%> (-13.76%) ⬇️
megalinter/reporters/UpdatedSourcesReporter.py 89.74% <0.00%> (+2.56%) ⬆️
...alinter/tests/test_megalinter/helpers/utilstest.py 89.01% <0.00%> (+8.05%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@nvuillam
Copy link
Member

Don't forget to build with bash build.sh, it will generate the test class :)

@bdovaz
Copy link
Collaborator Author

bdovaz commented Dec 26, 2022

@nvuillam done.

Can you read and answer me to the first message of this PR related to the tests?

I see that there are special cases where you exclude this type of test (linter_failure):

https://github.com/oxsecurity/megalinter/blob/v6.16.0/megalinter/tests/test_megalinter/helpers/utilstest.py#L214

If we don't find a solution I don't know if this would be the solution.

@nvuillam
Copy link
Member

@bdovaz do powershell formatter have a --check mode, to just check if the file is already formatted or not ?

@bdovaz
Copy link
Collaborator Author

bdovaz commented Dec 26, 2022

@nvuillam
Copy link
Member

@bdovaz it seems that Microsoft does not like to build their apps like all others ^^

In that case you can add an empty file named no_test_failure (without extension) in the test folder

Copy link
Member

@nvuillam nvuillam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plz check comment :)

- Add test_folder parameter
- Refactor test files
- Create no_test_failure file
@nvuillam
Copy link
Member

❌ Linted [REPOSITORY] files with [git_diff]: Found 1 error(s) - (0.03s)
- Using [git_diff v2.36.3] https://megalinter.io/2644adfa201ad6bd9bec5f21e8040cd699416064/descriptors/repository_git_diff
- MegaLinter key: [REPOSITORY_GIT_DIFF]
- Rules config: identified by [git_diff]
--Error detail:
.automation/test/powershell_formatter/good/powershell_good_1.ps1:2: new blank line at EOF.
.automation/test/powershell_formatter/good/powershell_good_1.psd1:4: new blank line at EOF.
.automation/test/powershell_formatter/good/powershell_good_1.psm1:2: new blank line at EOF.

That's really strange, I never saw this one :D
Maybe some error while mananing conflicts ?

@bdovaz
Copy link
Collaborator Author

bdovaz commented Dec 26, 2022

❌ Linted [REPOSITORY] files with [git_diff]: Found 1 error(s) - (0.03s)
- Using [git_diff v2.36.3] https://megalinter.io/2644adfa201ad6bd9bec5f21e8040cd699416064/descriptors/repository_git_diff
- MegaLinter key: [REPOSITORY_GIT_DIFF]
- Rules config: identified by [git_diff]
--Error detail:
.automation/test/powershell_formatter/good/powershell_good_1.ps1:2: new blank line at EOF.
.automation/test/powershell_formatter/good/powershell_good_1.psd1:4: new blank line at EOF.
.automation/test/powershell_formatter/good/powershell_good_1.psm1:2: new blank line at EOF.

That's really strange, I never saw this one :D Maybe some error while mananing conflicts ?

@nvuillam Well I don't know what to do to fix this because if you look at my PR I simply copied the 3 files from the powershell folder as is.

@nvuillam
Copy link
Member

@bdovaz what if you add a blank line in the files so they errors can disappear ?

@bdovaz
Copy link
Collaborator Author

bdovaz commented Dec 26, 2022

@nvuillam I just did it but that is the solution? I just don't understand the error either, what it refers to and how to fix it.

Let's wait for the result of the build.

@nvuillam nvuillam marked this pull request as ready for review December 26, 2022 18:53
@nvuillam
Copy link
Member

That's really really strange... as the rest seems legit, I force the PR, we'll see if it solves the issue ^^

@nvuillam nvuillam merged commit 2eb9c7d into oxsecurity:main Dec 26, 2022
@echoix
Copy link
Collaborator

echoix commented Dec 26, 2022

This reminds me of when git changes the end of lines when checking-out, and changing them back when committing. It is normally controlled by a .gitattributes file (like https://github.com/oxsecurity/megalinter/blob/main/.gitattributes ). Are you working on Windows?

@bdovaz
Copy link
Collaborator Author

bdovaz commented Dec 26, 2022

This reminds me of when git changes the end of lines when checking-out, and changing them back when committing. It is normally controlled by a .gitattributes file (like https://github.com/oxsecurity/megalinter/blob/main/.gitattributes ). Are you working on Windows?

@nvuillam @echoix yes, I'm on Windows.

@bdovaz bdovaz deleted the feature/powershell-formatter branch December 26, 2022 19:17
@nvuillam
Copy link
Member

I'm on windows too but never had the issue ^^

If it does not happen on this PR #2189 , let's consider the problem solved ^^

@nvuillam
Copy link
Member

@echoix
Copy link
Collaborator

echoix commented Dec 27, 2022

I'm on windows too but never had the issue ^^

Some of the .gitattributes won't get applied to an already cloned repo without any actions, so it doesn't exclude the problem.

https://docs.github.com/en/get-started/getting-started-with-git/configuring-git-to-handle-line-endings

https://git-scm.com/docs/gitattributes

@nvuillam
Copy link
Member

Finally solved... i just remove the line at the end of the files >_<

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 this pull request may close these issues.

None yet

4 participants