Refactor MoveUsingStatements

PoshCode · Jaykul · Aug 7, 2019 · Aug 1, 2019 · Aug 1, 2019 · Aug 1, 2019
commit ccfe81f46fc144c32ee8c9cc7eb1ce080c4f6e35
diff --git a/Source/Private/ConvertToAst.ps1 b/Source/Private/ConvertToAst.ps1
@@ -0,0 +1,36 @@
+function ConvertToAst {
+    <#
+        .SYNOPSIS
+            Parses the given code and returns an object with the AST, Tokens and ParseErrors
+    #>
+    param(
+        # The script content, or script or module file path to parse
+        [Parameter(Mandatory, ValueFromPipeline, ValueFromPipelineByPropertyName)]
+        [Alias("Path", "PSPath", "Definition", "ScriptBlock", "Module")]
+        $Code
+    )
+    process {
+        Write-Debug "    ENTER: ConvertToAst $Code"
+        $ParseErrors = $null
+        $Tokens = $null
+        if ($Code | Test-Path -ErrorAction SilentlyContinue) {
+            Write-Debug "      Parse Code as Path"
+            $AST = [System.Management.Automation.Language.Parser]::ParseFile(($Code | Convert-Path), [ref]$Tokens, [ref]$ParseErrors)
+        } elseif ($Code -is [System.Management.Automation.FunctionInfo]) {
+            Write-Debug "      Parse Code as Function"
+            $String = "function $($Code.Name) { $($Code.Definition) }"
+            $AST = [System.Management.Automation.Language.Parser]::ParseInput($String, [ref]$Tokens, [ref]$ParseErrors)
+        } else {
+            Write-Debug "      Parse Code as String"
+            $AST = [System.Management.Automation.Language.Parser]::ParseInput([String]$Code, [ref]$Tokens, [ref]$ParseErrors)
+        }
+
+        Write-Debug "    EXIT: ConvertToAst"
+        [PSCustomObject]@{
+            PSTypeName  = "PoshCode.ModuleBuilder.ParseResults"
+            ParseErrors = $ParseErrors
+            Tokens      = $Tokens
+            AST         = $AST
+        }
+    }
+}
diff --git a/Source/Private/MoveUsingStatements.ps1 b/Source/Private/MoveUsingStatements.ps1
@@ -19,27 +19,24 @@ function MoveUsingStatements {
     Param(
         # Path to the PSM1 file to amend
         [Parameter(Mandatory, ValueFromPipelineByPropertyName, ValueFromPipeline)]
-        $RootModule,
+        [System.Management.Automation.Language.Ast]$AST,
+
+        [Parameter(ValueFromPipelineByPropertyName, ValueFromPipeline)]
+        [AllowNull()]
+        [System.Management.Automation.Language.ParseError[]]$ParseErrors,
 
         # The encoding defaults to UTF8 (or UTF8NoBom on Core)
         [Parameter(DontShow)]
         [string]$Encoding = $(if ($IsCoreCLR) { "UTF8NoBom" } else { "UTF8" })
     )
 
-    $ParseError = $null
-    $AST = [System.Management.Automation.Language.Parser]::ParseFile(
-        $RootModule,
-        [ref]$null,
-        [ref]$ParseError
-    )
-
     # Avoid modifying the file if there's no Parsing Error caused by Using Statements or other errors
-    if (!$ParseError.Where{$_.ErrorId -eq 'UsingMustBeAtStartOfScript'}) {
+    if (!$ParseErrors.Where{$_.ErrorId -eq 'UsingMustBeAtStartOfScript'}) {
         Write-Debug "No Using Statement Error found."
         return
     }
     # Avoid modifying the file if there's other parsing errors than Using Statements misplaced
-    if ($ParseError.Where{$_.ErrorId -ne 'UsingMustBeAtStartOfScript'}) {
+    if ($ParseErrors.Where{$_.ErrorId -ne 'UsingMustBeAtStartOfScript'}) {
         Write-Warning "Parsing errors found. Skipping Moving Using statements."
         return
     }
@@ -70,13 +67,13 @@ function MoveUsingStatements {
     $null = [System.Management.Automation.Language.Parser]::ParseInput(
         $ScriptText,
         [ref]$null,
-        [ref]$ParseError
+        [ref]$ParseErrors
     )
 
-    if ($ParseError) {
+    if ($ParseErrors) {
         Write-Warning "Oops, it seems that we introduced parsing error(s) while moving the Using Statements. Cancelling changes."
     }
     else {
         $null = Set-Content -Value $ScriptText -Path $RootModule -Encoding $Encoding
     }
-}
+}
diff --git a/Source/Public/Build-Module.ps1 b/Source/Public/Build-Module.ps1
@@ -42,6 +42,7 @@ function Build-Module {
     [Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSUseApprovedVerbs", "", Justification="Build is approved now")]
     [Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSUseCmdletCorrectly", "")]
     [CmdletBinding(DefaultParameterSetName="SemanticVersion")]
+    [Alias("build")]
     param(
         # The path to the module folder, manifest or build.psd1
         [Parameter(Position = 0, ValueFromPipelineByPropertyName)]
@@ -96,7 +97,7 @@ function Build-Module {
         ),
 
         # A Filter (relative to the module folder) for public functions
-        # If non-empty, ExportedFunctions will be set with the file BaseNames of matching files
+        # If non-empty, FunctionsToExport will be set with the file BaseNames of matching files
         # Defaults to Public\*.ps1
         [AllowEmptyString()]
         [string[]]$PublicFilter = "Public\*.ps1",
@@ -197,17 +198,19 @@ function Build-Module {
             # SilentlyContinue because there don't *HAVE* to be functions at all
             $AllScripts = Get-ChildItem -Path @($ModuleInfo.SourceDirectories).ForEach{ Join-Path $ModuleInfo.ModuleBase $_ } -Filter *.ps1 -Recurse -ErrorAction SilentlyContinue
 
+            # We have to force the Encoding to string because PowerShell Core made up encodings
             SetModuleContent -Source (@($ModuleInfo.Prefix) + $AllScripts.FullName + @($ModuleInfo.Suffix)).Where{$_} -Output $RootModule -Encoding "$($ModuleInfo.Encoding)"
-            MoveUsingStatements -RootModule $RootModule -Encoding "$($ModuleInfo.Encoding)"
 
             # If there is a PublicFilter, update ExportedFunctions
             if ($ModuleInfo.PublicFilter) {
                 # SilentlyContinue because there don't *HAVE* to be public functions
-                if ($PublicFunctions = Get-ChildItem $ModuleInfo.PublicFilter -Recurse -ErrorAction SilentlyContinue | Select-Object -ExpandProperty BaseName) {
+                if (($PublicFunctions = Get-ChildItem $ModuleInfo.PublicFilter -Recurse -ErrorAction SilentlyContinue | Select-Object -ExpandProperty BaseName)) {
                     Update-Metadata -Path $OutputManifest -PropertyName FunctionsToExport -Value $PublicFunctions
                 }
             }
 
+            $ParseResult = ConvertToAst $RootModule
+            $ParseResult | MoveUsingStatements -Encoding "$($ModuleInfo.Encoding)"
             try {
                 if ($Version) {
                     Write-Verbose "Update Manifest at $OutputManifest with version: $Version"

diff --git a/Tests/Private/ConvertToAst.Tests.ps1 b/Tests/Private/ConvertToAst.Tests.ps1
@@ -0,0 +1,78 @@
+Describe "ConvertToAst" {
+    Import-Module ModuleBuilder -DisableNameChecking -Verbose:$False
+
+    Context "It returns a ParseResult for file paths" {
+        $ParseResult = InModuleScope ModuleBuilder {
+            ConvertToAst $PSCommandPath -Debug
+        }
+
+        It "Returns a ParseResult object" {
+            $ParseResult.PSTypeNames[0] | Should -Match .*\.ParseResult
+        }
+        It "Has an AST property" {
+            $ParseResult.AST | Should -BeOfType [System.Management.Automation.Language.Ast]
+        }
+        It "Has a ParseErrors property" {
+            $ParseResult.ParseErrors | Should -BeNullOrEmpty # [System.Management.Automation.Language.ParseError[]]
+        }
+        It "Has a Tokens property" {
+            $ParseResult.Tokens | Should -BeOfType [System.Management.Automation.Language.Token]
+        }
+
+    }
+
+    Context "It parses piped in commands" {
+        $ParseResult = InModuleScope ModuleBuilder {
+            Get-Command ConvertToAst | ConvertToAst -Debug
+        }
+
+        It "Returns a ParseResult object with the AST" {
+            $ParseResult.PSTypeNames[0] | Should -Match .*\.ParseResult
+            $ParseResult.AST | Should -BeOfType [System.Management.Automation.Language.Ast]
+        }
+    }
+
+    Context "It parses piped in modules" {
+        $ParseResult = InModuleScope ModuleBuilder {
+            Get-Module ModuleBuilder | ConvertToAst -Debug
+        }
+
+        It "Returns a ParseResult object with the AST" {
+            $ParseResult.PSTypeNames[0] | Should -Match .*\.ParseResult
+            $ParseResult.AST | Should -BeOfType [System.Management.Automation.Language.Ast]
+        }
+    }
+
+<#
+        param(
+        # The script content, or script or module file path to parse
+        [Parameter(Mandatory, ValueFromPipeline, ValueFromPipelineByPropertyName)]
+        [Alias("Path", "PSPath", "Definition", "ScriptBlock", "Module")]
+        $Code
+    )
+    process {
+        Write-Debug "    ENTER: ConvertToAst $Code"
+        $ParseErrors = $null
+        $Tokens = $null
+        if ($Code | Test-Path -ErrorAction SilentlyContinue) {
+            Write-Debug "      Parse Code as Path"
+            $AST = [System.Management.Automation.Language.Parser]::ParseFile(($Code | Convert-Path), [ref]$Tokens, [ref]$ParseErrors)
+        } elseif ($Code -is [System.Management.Automation.FunctionInfo]) {
+            Write-Debug "      Parse Code as Function"
+            $String = "function $($Code.Name) { $($Code.Definition) }"
+            $AST = [System.Management.Automation.Language.Parser]::ParseInput($String, [ref]$Tokens, [ref]$ParseErrors)
+        } else {
+            Write-Debug "      Parse Code as String"
+            $AST = [System.Management.Automation.Language.Parser]::ParseInput([String]$Code, [ref]$Tokens, [ref]$ParseErrors)
+        }
+
+        Write-Debug "    EXIT: ConvertToAst"
+        [PSCustomObject]@{
+            PSTypeName  = "PoshCode.ModuleBuilder.ParseResults"
+            ParseErrors = $ParseErrors
+            Tokens      = $Tokens
+            AST         = $AST
+        }
+    }
+    #>
+}
diff --git a/Tests/Private/MoveUsingStatements.Tests.ps1 b/Tests/Private/MoveUsingStatements.Tests.ps1
@@ -3,10 +3,11 @@ Describe "MoveUsingStatements" {
     Context "Necessary Parameters" {
         $CommandInfo = InModuleScope ModuleBuilder { Get-Command MoveUsingStatements }
 
-        It 'has a mandatory RootModule parameter' {
-            $RootModule = $CommandInfo.Parameters['RootModule']
-            $RootModule | Should -Not -BeNullOrEmpty
-            $RootModule.Attributes.Where{$_ -is [Parameter]}.Mandatory | Should -Be $true
+        It 'has a mandatory AST parameter' {
+            $AST = $CommandInfo.Parameters['AST']
+            $AST | Should -Not -BeNullOrEmpty
+            $AST.ParameterType | Should -Be ([System.Management.Automation.Language.Ast])
+            $AST.Attributes.Where{ $_ -is [Parameter] }.Mandatory | Should -Be $true
         }
 
         It "has an optional string Encoding parameter" {
@@ -18,42 +19,54 @@ Describe "MoveUsingStatements" {
     }
 
     Context "Moving Using Statements to the beginning of the file" {
+
         $MoveUsingStatementsCmd = InModuleScope ModuleBuilder {
-            $null = Mock Write-Warning {}
-            Get-Command MoveUsingStatements
+            $null = Mock Write-Warning { }
+            {   param($RootModule)
+                ConvertToAst $RootModule | MoveUsingStatements
+            }
         }
 
         $TestCases = @(
             @{
-                TestCaseName = '2xUsingMustBeAtStartOfScript Fixed'
+                TestCaseName = 'Move all using statements in `n terminated files to the top'
+                PSM1File     = "function x {`n}`n" +
+                "using namespace System.IO`n`n" + #UsingMustBeAtStartOfScript
+                "function y {`n}`n" +
+                "using namespace System.Drawing" #UsingMustBeAtStartOfScript
+                ErrorBefore  = 2
+                ErrorAfter   = 0
+            },
+            @{
+                TestCaseName = 'Move all using statements in `r`n terminated files to the top'
                 PSM1File     = "function x {`r`n}`r`n" +
-                "Using namespace System.io`r`n`r`n" + #UsingMustBeAtStartOfScript
+                "USING namespace System.IO`r`n`r`n" + #UsingMustBeAtStartOfScript
                 "function y {`r`n}`r`n" +
-                "Using namespace System.Drawing" #UsingMustBeAtStartOfScript
+                "USING namespace System.Drawing" #UsingMustBeAtStartOfScript
                 ErrorBefore  = 2
                 ErrorAfter   = 0
             },
             @{
-                TestCaseName = 'NoErrors Do Nothing'
-                PSM1File     = "Using namespace System.io`r`n`r`n" +
-                "Using namespace System.Drawing`r`n" +
+                TestCaseName = 'Not change the content again if there are no out-of-place using statements'
+                PSM1File     = "using namespace System.IO`r`n`r`n" +
+                "using namespace System.Drawing`r`n" +
                 "function x { `r`n}`r`n" +
                 "function y { `r`n}`r`n"
                 ErrorBefore  = 0
                 ErrorAfter   = 0
             },
             @{
-                TestCaseName = 'NotValidPowerShel Do Nothing'
-                PSM1File     = "Using namespace System.io`r`n`r`n" +
+                TestCaseName = 'Not move anything if there are (other) parse errors'
+                PSM1File     = "using namespace System.IO`r`n`r`n" +
                 "function x { `r`n}`r`n" +
-                "Using namespace System.Drawing`r`n" + # UsingMustBeAtStartOfScript
+                "using namespace System.Drawing`r`n" + # UsingMustBeAtStartOfScript
                 "function y { `r`n}`r`n}" # Extra } at the end
                 ErrorBefore  = 2
                 ErrorAfter   = 2
             }
         )
 
-        It 'Should succeed test: "<TestCaseName>" from <ErrorBefore> to <ErrorAfter> parsing errors' -TestCases $TestCases {
+        It 'It should <TestCaseName>' -TestCases $TestCases {
             param($TestCaseName, $PSM1File, $ErrorBefore, $ErrorAfter)
 
             $testModuleFile = "$TestDrive/MyModule.psm1"
@@ -85,7 +98,9 @@ Describe "MoveUsingStatements" {
             $null = Mock Set-Content {}
             $null = Mock Write-Debug {} -ParameterFilter {$Message -eq "No Using Statement Error found." }
 
-            Get-Command MoveUsingStatements
+            {   param($RootModule)
+                ConvertToAst $RootModule | MoveUsingStatements
+            }
         }
 
         It 'Should Warn and skip when there are Parsing errors other than Using Statements' {

diff --git a/Tests/Public/Convert-CodeCoverage.Tests.ps1 b/Tests/Public/Convert-CodeCoverage.Tests.ps1
@@ -16,9 +16,6 @@ Describe "Convert-CodeCoverage" {
         $PesterResults = [PSCustomObject]@{
             CodeCoverage = [PSCustomObject]@{
                 MissedCommands = [PSCustomObject]@{
-                    # Note these don't really matter
-                    Command = $ModuleContent[25]
-                    Function = 'CopyReadMe'
                     # these are pipeline bound
                     File = $ModulePath
                     Line = 26 # 1 offset with the Using Statement introduced in MoveUsingStatements
@@ -28,7 +25,8 @@ Describe "Convert-CodeCoverage" {
 
         $SourceLocation = $PesterResults | Convert-CodeCoverage -SourceRoot $ModuleSource
 
-        $SourceLocation.SourceFile | Should -Be ".\Private\CopyReadMe.ps1"
+        # Needs to match the actual module source (on line 25)
+        $SourceLocation.SourceFile | Should -Be ".\Private\ConvertToAst.ps1"
         $SourceLocation.Line | Should -Be 25
     }
 }
diff --git a/Tests/Public/Convert-LineNumber.Tests.ps1 b/Tests/Public/Convert-LineNumber.Tests.ps1
@@ -51,18 +51,17 @@ Describe "Convert-LineNumber" {
 
     It 'Should pass through InputObject for updating objects like CodeCoverage or ErrorRecord' {
         $PesterMiss = [PSCustomObject]@{
-            # Note these don't really matter
-            Command = $ModuleContent[25]
-            Function = 'CopyReadMe'
+            # Note these don't really matter, but they're passed through
+            Function = 'TotalNonsense'
             # these are pipeline bound
             File = $ModulePath
             Line = 26 # 1 offset with the Using Statement introduced in MoveUsingStatements
         }
 
         $SourceLocation = $PesterMiss | Convert-LineNumber -Passthru
         # This test is assuming you built the code on Windows. Should Convert-LineNumber convert the path?
-        $SourceLocation.SourceFile | Should -Be ".\Private\CopyReadMe.ps1"
+        $SourceLocation.SourceFile | Should -Be ".\Private\ConvertToAst.ps1"
         $SourceLocation.SourceLineNumber | Should -Be 25
-        $SourceLocation.Function | Should -Be 'CopyReadMe'
+        $SourceLocation.Function | Should -Be 'TotalNonsense'
     }
 }