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.

Sign up for GitHub

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

Jump to bottom

Add checks of basic PSScriptAnalyzer rules #700

Closed
nohwnd opened this issue Jan 22, 2017 · 8 comments
Closed

Add checks of basic PSScriptAnalyzer rules #700

nohwnd opened this issue Jan 22, 2017 · 8 comments
Labels
Feature

Comments

@nohwnd
Copy link
Member

nohwnd commented Jan 22, 2017

Add PSScriptAnalyzer analyzer rules to be used as a part of pre-release checks:

  • PSAvoidUsingCmdletAliases
  • PSUseDeclaredVarsMoreThanAssignments
  • PSUseOutputTypeCorrectly
  • PSAvoidDefaultValueForMandatoryParameter
  • PSUseCmdletCorrectly

reported by @it-praktyk
@nohwnd nohwnd added the Feature label Jan 22, 2017
@nohwnd nohwnd added this to the V4 milestone Jan 22, 2017
@it-praktyk
Copy link
Contributor

You can assign it to me.

@it-praktyk it-praktyk mentioned this issue Jan 22, 2017
Closed
@it-praktyk
Copy link
Contributor

@nohwnd , please change the title of the issue to: "Add checks of basic PSScriptAnalyzer rules"

The current results of added tests

Describing PSScriptAnalyzer rules
   [-] Pester source code doesn't violate the PSScriptAnalzyer rule PSAvoidUsingCmdletAliases 1.44s
     The following 52 lines violate PSScriptAnalyzer rule PSAvoidUsingCmdletAliases:

     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\build.psake.ps1, Line: 26
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\build.psake.ps1, Line: 27
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Pester.psm1, Line: 175
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Pester.psm1, Line: 190
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Assertions\Should.Tests.ps1, Line: 112

     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 182
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 182
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 182
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 183
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 186
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 186
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 186
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 187
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 252
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 288
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 295
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 300
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 314
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 355
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 395
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 395
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 448
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 448
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 456
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\GherkinHook.Tests.ps1, Line: 31
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\GherkinHook.Tests.ps1, Line: 38
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\GherkinHook.Tests.ps1, Line: 43
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\GherkinStep.Tests.ps1, Line: 31
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.Tests.ps1, Line: 144
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.Tests.ps1, Line: 275
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.Tests.ps1, Line: 1320
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.Tests.ps1, Line: 1415
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.Tests.ps1, Line: 1425
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.Tests.ps1, Line: 1434
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.Tests.ps1, Line: 1443
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.Tests.ps1, Line: 1466
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\New-Fixture.Tests.ps1, Line: 13
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\New-Fixture.Tests.ps1, Line: 15
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\New-Fixture.Tests.ps1, Line: 43
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\New-Fixture.Tests.ps1, Line: 45
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\New-Fixture.Tests.ps1, Line: 54
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\New-Fixture.Tests.ps1, Line: 56
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\New-Fixture.Tests.ps1, Line: 65
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\New-Fixture.Tests.ps1, Line: 66
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\New-Fixture.Tests.ps1, Line: 67
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Output.ps1, Line: 102
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Output.ps1, Line: 126
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\TestResults.Tests.ps1, Line: 272
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\TestResults.Tests.ps1, Line: 274
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\TestResults.Tests.ps1, Line: 279
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\TestResults.Tests.ps1, Line: 282
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\TestsRunningInCleanRunspace.Tests.ps1,
Line: 30
     at line: 206 in C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Pester.Tests.ps1
   [-] Pester source code doesn't violate the PSScriptAnalzyer rule PSUseDeclaredVarsMoreThanAssignments 1.49s
     The following 29 lines violate PSScriptAnalyzer rule PSUseDeclaredVarsMoreThanAssignments:

     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\build.psake.ps1, Line: 3
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\build.psake.ps1, Line: 4
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\build.psake.ps1, Line: 6
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\build.psake.ps1, Line: 7
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\build.psake.ps1, Line: 8
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Pester.psm1, Line: 299
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Pester.Tests.ps1, Line: 89
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Assertions\Should.Tests.ps1, Line: 89
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Assertions\Should.Tests.ps1, Line: 90
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 5
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 257
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.ps1, Line: 834
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.Tests.ps1, Line: 275
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.Tests.ps1, Line: 1600
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Output.ps1, Line: 293
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\PesterState.ps1, Line: 43
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\PesterState.ps1, Line: 44
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\PesterState.ps1, Line: 45
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\PesterState.Tests.ps1, Line: 166
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\SetupTeardown.Tests.ps1, Line: 3
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\SetupTeardown.Tests.ps1, Line: 23
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\SetupTeardown.Tests.ps1, Line: 40
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\SetupTeardown.Tests.ps1, Line: 49
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\SetupTeardown.Tests.ps1, Line: 53
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\SetupTeardown.Tests.ps1, Line: 66
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\SetupTeardown.Tests.ps1, Line: 82
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\SetupTeardown.Tests.ps1, Line: 87
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\SetupTeardown.Tests.ps1, Line: 98
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\SetupTeardown.Tests.ps1, Line: 167
     at line: 206 in C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Pester.Tests.ps1
   [-] Pester source code doesn't violate the PSScriptAnalzyer rule PSUseOutputTypeCorrectly 1.44s
     The following 16 lines violate PSScriptAnalyzer rule PSUseOutputTypeCorrectly:

     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Pester.psm1, Line: 946
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Pester.psm1, Line: 950
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Pester.psm1, Line: 954
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Pester.psm1, Line: 969
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.ps1, Line: 1464
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.Tests.ps1, Line: 12
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.Tests.ps1, Line: 917
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.Tests.ps1, Line: 983
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.Tests.ps1, Line: 1041
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.Tests.ps1, Line: 1098
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.Tests.ps1, Line: 1149
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.Tests.ps1, Line: 1203
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.Tests.ps1, Line: 1268
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.Tests.ps1, Line: 1361
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.Tests.ps1, Line: 1557
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Mock.Tests.ps1, Line: 1565
     at line: 206 in C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Pester.Tests.ps1
   [-] Pester source code doesn't violate the PSScriptAnalzyer rule PSAvoidDefaultValueForMandatoryParameter 1.36s
     The following 1 lines violate PSScriptAnalyzer rule PSAvoidDefaultValueForMandatoryParameter:

     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\GherkinHook.ps1, Line: 5
     at line: 206 in C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Pester.Tests.ps1
   [-] Pester source code doesn't violate the PSScriptAnalzyer rule PSUseCmdletCorrectly 51.7s
     The following 2 lines violate PSScriptAnalyzer rule PSUseCmdletCorrectly:

     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 283
     File: C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Functions\Gherkin.ps1, Line: 474
     at line: 206 in C:\Users\Wojtek\Documents\Scripts\2 - GitHub Extenal\Pester\Pester.Tests.ps1

@it-praktyk
Copy link
Contributor

I'm not sure if we can use PSScriptAnalyzer rules just now.

Please read the issue PowerShell/PSScriptAnalyzer#636.

@it-praktyk
Copy link
Contributor

I started clean code for violation reported by PSScriptAnalyzer tool - results are now in https://github.com/it-praktyk/Pester/tree/DevV4_PSScriptAnalyzer_Clean_PSAvoidUsingCmdletAliases

@dlwyatt , could you check?

    [-] Pester source code doesn't violate the PSScriptAnalzyer rule PSAvoidUsingCmdletAliases 1.83s
      The following 1 lines violate PSScriptAnalyzer rule PSAvoidUsingCmdletAliases:

      File: C:\Users\<SOME DIR>\Pester\Functions\Mock.Tests.ps1, Line: 144
      at line: 206 in C:\Users\<SOME DIR>\Pester\Pester.Tests.ps1

@nohwnd nohwnd changed the title Add basic style rules checks Add checks of basic PSScriptAnalyzer rules Jan 23, 2017
@Jaykul
Copy link
Contributor

Jaykul commented Jan 24, 2017

Oh good grief, now we're going to have to spell out "Where-Object" lest anyone think we meant some other "Where" ... 🙄

@nohwnd
Copy link
Member Author

nohwnd commented Jan 25, 2017

@it-praktyk I think it is contra-productive to start cleaning the code at the moment, we have more pressing issues to fix and changing the style will make merging harder. At the moment we only need to identify issues in the public (exported) functions that would cause change of the public API if fixed. Did you see any of those?

@it-praktyk it-praktyk mentioned this issue Jan 25, 2017
Closed
@it-praktyk
Copy link
Contributor

Yes, I agree that clean of code is less important than finding/fixing functional issues etc.

In this moment I don't see any issue related to PSScriptAnalyzer warnings what fixed can have impact for public API.
The scope of proposed to use rules is small and don't include e.g.

  • PSUseSingularNouns
  • PSUseApprovedVerbs

@nohwnd nohwnd removed this from the V4 milestone Mar 27, 2017
@it-praktyk it-praktyk mentioned this issue Mar 30, 2017
Merged
@it-praktyk
Copy link
Contributor

Due to specific of Pester (especially mocking) usage of PSScriptAnalyzer is not a good idea. Some cleaning was done already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@Jaykul @it-praktyk @nohwnd