From fd787e114f2de739addcdd9aee62a8560fbdf144 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 31 Mar 2018 17:29:02 +0100 Subject: [PATCH 01/16] Trim out TypeNotFoundParseErrors. --- Engine/ScriptAnalyzer.cs | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 67ff44e73..620e1f234 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1522,16 +1522,18 @@ public IEnumerable AnalyzeScriptDefinition(string scriptDefini return null; } - if (errors != null && errors.Length > 0) + var relevantParseErrors = RemoveTypeNotFoundParseErrors(errors); + + if (relevantParseErrors != null && relevantParseErrors.Count > 0) { - foreach (ParseError error in errors) + foreach (ParseError error in relevantParseErrors) { string parseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParseErrorFormatForScriptDefinition, error.Message.TrimEnd('.'), error.Extent.StartLineNumber, error.Extent.StartColumnNumber); this.outputWriter.WriteError(new ErrorRecord(new ParseException(parseErrorMessage), parseErrorMessage, ErrorCategory.ParserError, error.ErrorId)); } } - if (errors != null && errors.Length > 10) + if (relevantParseErrors != null && relevantParseErrors.Count > 10) { string manyParseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParserErrorMessageForScriptDefinition); this.outputWriter.WriteError(new ErrorRecord(new ParseException(manyParseErrorMessage), manyParseErrorMessage, ErrorCategory.ParserError, scriptDefinition)); @@ -1644,6 +1646,26 @@ private static Encoding GetFileEncoding(string path) } } + /// + /// Inspects Parse errors and removes TypeNotFound errors that can be ignored that can be due to types that are not known yet (e.g. due to 'using' statements) + /// + /// + /// List of relevant parse errors. + private List RemoveTypeNotFoundParseErrors(ParseError[] parseErrors) + { + var relevantParseErrors = new List(); + foreach(var parseError in parseErrors) + { + // If types are not known due them not being imported yet, the parser throws an error that can be ignored + if (parseError.ErrorId != "TypeNotFound") + { + relevantParseErrors.Add(parseError); + } + } + + return relevantParseErrors; + } + private static Range SnapToEdges(EditableText text, Range range) { // todo add TextLines.Validate(range) and TextLines.Validate(position) @@ -1829,17 +1851,19 @@ private IEnumerable AnalyzeFile(string filePath) scriptAst = Parser.ParseFile(filePath, out scriptTokens, out errors); } #endif //!PSV3 + var relevantParseErrors = RemoveTypeNotFoundParseErrors(errors); + //Runspace.DefaultRunspace = oldDefault; - if (errors != null && errors.Length > 0) + if (relevantParseErrors != null && relevantParseErrors.Count > 0) { - foreach (ParseError error in errors) + foreach (ParseError error in relevantParseErrors) { string parseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParserErrorFormat, error.Extent.File, error.Message.TrimEnd('.'), error.Extent.StartLineNumber, error.Extent.StartColumnNumber); this.outputWriter.WriteError(new ErrorRecord(new ParseException(parseErrorMessage), parseErrorMessage, ErrorCategory.ParserError, error.ErrorId)); } } - if (errors != null && errors.Length > 10) + if (relevantParseErrors != null && relevantParseErrors.Count > 10) { string manyParseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParserErrorMessage, System.IO.Path.GetFileName(filePath)); this.outputWriter.WriteError(new ErrorRecord(new ParseException(manyParseErrorMessage), manyParseErrorMessage, ErrorCategory.ParserError, filePath)); From 5d2f5d4090b0a0f36d0098af2379edf9a9573f52 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 31 Mar 2018 17:40:14 +0100 Subject: [PATCH 02/16] Add verbose logging and test --- Engine/ScriptAnalyzer.cs | 5 +++++ Tests/Engine/InvokeScriptAnalyzer.tests.ps1 | 14 ++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 620e1f234..524226540 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1661,6 +1661,11 @@ private List RemoveTypeNotFoundParseErrors(ParseError[] parseErrors) { relevantParseErrors.Add(parseError); } + else + { + this.outputWriter.WriteVerbose( + $"Ignoring 'TypeNotFound' parse error on type {parseError.Extent}, which is likely due to the type not being knowm due to e.g. 'using' statements. Parse error message was '{parseError.Message}'."); + } } return relevantParseErrors; diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 index 09f39243c..491564893 100644 --- a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 +++ b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 @@ -562,4 +562,18 @@ Describe "Test -EnableExit Switch" { "$result" | Should -Not -BeLike $reportSummaryFor1Warning } } + + Describe "Handles parse errors due to unknown types" { + $script = @' + using namespace Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkModels + using namespace Microsoft.Azure.Commands.Common.Authentication.Abstractions + Import-Module "AzureRm" + class MyClass { [IStorageContext]$StorageContext } + gci # Produce AvoidAlias rule +'@ + It { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition $script + $warnings.Count | Should -Be 1 -Because 'PSSA should analyze the whole script after the parse error on [IStorageContext] and find the AvoidAlias warning due to gci' + } + } } From 8a2826ebb3b98450cfa6f78bfd6158c989d92d96 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 31 Mar 2018 17:49:40 +0100 Subject: [PATCH 03/16] fix test syntax --- Tests/Engine/InvokeScriptAnalyzer.tests.ps1 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 index 491564893..449e5cb2c 100644 --- a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 +++ b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 @@ -568,10 +568,10 @@ Describe "Test -EnableExit Switch" { using namespace Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkModels using namespace Microsoft.Azure.Commands.Common.Authentication.Abstractions Import-Module "AzureRm" - class MyClass { [IStorageContext]$StorageContext } + class MyClass { [IStorageContext]$StorageContext } # This will result in a parser error due to [IStorageContext] type that comes from the using stetement but is not known at parse time gci # Produce AvoidAlias rule '@ - It { + It "does not throw and detect one expected warning after the parse error has occured" { $warnings = Invoke-ScriptAnalyzer -ScriptDefinition $script $warnings.Count | Should -Be 1 -Because 'PSSA should analyze the whole script after the parse error on [IStorageContext] and find the AvoidAlias warning due to gci' } From a24e185790fe0c9531eab67d558f22576a21a657 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 31 Mar 2018 17:58:01 +0100 Subject: [PATCH 04/16] add test for -path parameter set --- Tests/Engine/InvokeScriptAnalyzer.tests.ps1 | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 index 449e5cb2c..86999f683 100644 --- a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 +++ b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 @@ -571,9 +571,21 @@ Describe "Test -EnableExit Switch" { class MyClass { [IStorageContext]$StorageContext } # This will result in a parser error due to [IStorageContext] type that comes from the using stetement but is not known at parse time gci # Produce AvoidAlias rule '@ - It "does not throw and detect one expected warning after the parse error has occured" { + It "does not throw and detect one expected warning after the parse error has occured when using -ScriptDefintion parameter set" { $warnings = Invoke-ScriptAnalyzer -ScriptDefinition $script $warnings.Count | Should -Be 1 -Because 'PSSA should analyze the whole script after the parse error on [IStorageContext] and find the AvoidAlias warning due to gci' } + + It "does not throw and detect one expected warning after the parse error has occured when using -Path parameter set" { + try { + $testFilePath = (Join-Path $PWD 'testFile.ps1') + $script > $testFilePath + $warnings = Invoke-ScriptAnalyzer -Path $testFilePath + $warnings.Count | Should -Be 1 -Because 'PSSA should analyze the whole script after the parse error on [IStorageContext] and find the AvoidAlias warning due to gci' + } + finally { + Remove-Item $testFilePath + } + } } } From bc3133267e14a62aca7d1244f65f352ac2c898b8 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 31 Mar 2018 18:03:47 +0100 Subject: [PATCH 05/16] use pester test drive --- Tests/Engine/InvokeScriptAnalyzer.tests.ps1 | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 index 86999f683..0a14c56ae 100644 --- a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 +++ b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 @@ -576,16 +576,11 @@ Describe "Test -EnableExit Switch" { $warnings.Count | Should -Be 1 -Because 'PSSA should analyze the whole script after the parse error on [IStorageContext] and find the AvoidAlias warning due to gci' } + $testFilePath = "TestDrive:\testfile.ps1" + Set-Content $testFilePath -value $script It "does not throw and detect one expected warning after the parse error has occured when using -Path parameter set" { - try { - $testFilePath = (Join-Path $PWD 'testFile.ps1') - $script > $testFilePath - $warnings = Invoke-ScriptAnalyzer -Path $testFilePath - $warnings.Count | Should -Be 1 -Because 'PSSA should analyze the whole script after the parse error on [IStorageContext] and find the AvoidAlias warning due to gci' - } - finally { - Remove-Item $testFilePath - } + $warnings = Invoke-ScriptAnalyzer -Path $testFilePath + $warnings.Count | Should -Be 1 -Because 'PSSA should analyze the whole script after the parse error on [IStorageContext] and find the AvoidAlias warning due to gci' } } } From e4dc844d367bcc7e9cdf38e144cd92d435f34415 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 31 Mar 2018 18:36:51 +0100 Subject: [PATCH 06/16] fix tests to not run in library usage session --- Tests/Engine/InvokeScriptAnalyzer.tests.ps1 | 34 +++++++++++---------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 index 0a14c56ae..e16bc34d4 100644 --- a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 +++ b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 @@ -563,24 +563,26 @@ Describe "Test -EnableExit Switch" { } } - Describe "Handles parse errors due to unknown types" { - $script = @' - using namespace Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkModels - using namespace Microsoft.Azure.Commands.Common.Authentication.Abstractions - Import-Module "AzureRm" - class MyClass { [IStorageContext]$StorageContext } # This will result in a parser error due to [IStorageContext] type that comes from the using stetement but is not known at parse time - gci # Produce AvoidAlias rule + if (!$testingLibraryUsage) { + Describe "Handles parse errors due to unknown types" { + $script = @' + using namespace Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkModels + using namespace Microsoft.Azure.Commands.Common.Authentication.Abstractions + Import-Module "AzureRm" + class MyClass { [IStorageContext]$StorageContext } # This will result in a parser error due to [IStorageContext] type that comes from the using stetement but is not known at parse time + gci # Produce AvoidAlias rule '@ - It "does not throw and detect one expected warning after the parse error has occured when using -ScriptDefintion parameter set" { - $warnings = Invoke-ScriptAnalyzer -ScriptDefinition $script - $warnings.Count | Should -Be 1 -Because 'PSSA should analyze the whole script after the parse error on [IStorageContext] and find the AvoidAlias warning due to gci' - } + It "does not throw and detect one expected warning after the parse error has occured when using -ScriptDefintion parameter set" { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition $script + $warnings.Count | Should -Be 1 -Because 'PSSA should analyze the whole script after the parse error on [IStorageContext] and find the AvoidAlias warning due to gci' + } - $testFilePath = "TestDrive:\testfile.ps1" - Set-Content $testFilePath -value $script - It "does not throw and detect one expected warning after the parse error has occured when using -Path parameter set" { - $warnings = Invoke-ScriptAnalyzer -Path $testFilePath - $warnings.Count | Should -Be 1 -Because 'PSSA should analyze the whole script after the parse error on [IStorageContext] and find the AvoidAlias warning due to gci' + $testFilePath = "TestDrive:\testfile.ps1" + Set-Content $testFilePath -value $script + It "does not throw and detect one expected warning after the parse error has occured when using -Path parameter set" { + $warnings = Invoke-ScriptAnalyzer -Path $testFilePath + $warnings.Count | Should -Be 1 -Because 'PSSA should analyze the whole script after the parse error on [IStorageContext] and find the AvoidAlias warning due to gci' + } } } } From d54d53199ea69ad00ef27dbd0485f0e495c5e603 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Thu, 5 Apr 2018 08:16:10 +0100 Subject: [PATCH 07/16] Address PR comments about additional assertion and xml comment --- Engine/ScriptAnalyzer.cs | 2 +- Tests/Engine/InvokeScriptAnalyzer.tests.ps1 | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 524226540..fa60c9967 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1647,7 +1647,7 @@ private static Encoding GetFileEncoding(string path) } /// - /// Inspects Parse errors and removes TypeNotFound errors that can be ignored that can be due to types that are not known yet (e.g. due to 'using' statements) + /// Inspects Parse errors and removes TypeNotFound errors that can be ignored since some types are not known yet (e.g. due to 'using' statements). /// /// /// List of relevant parse errors. diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 index e16bc34d4..8e843e70a 100644 --- a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 +++ b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 @@ -572,16 +572,19 @@ Describe "Test -EnableExit Switch" { class MyClass { [IStorageContext]$StorageContext } # This will result in a parser error due to [IStorageContext] type that comes from the using stetement but is not known at parse time gci # Produce AvoidAlias rule '@ + $assertionMessage = 'PSSA should analyze the whole script after the parse error on [IStorageContext] and find the PSAvoidUsingCmdletAliases warning due to gci' It "does not throw and detect one expected warning after the parse error has occured when using -ScriptDefintion parameter set" { $warnings = Invoke-ScriptAnalyzer -ScriptDefinition $script - $warnings.Count | Should -Be 1 -Because 'PSSA should analyze the whole script after the parse error on [IStorageContext] and find the AvoidAlias warning due to gci' + $warnings.Count | Should -Be 1 -Because $assertionMessage + $warnings.RuleName | Should -Be 'PSAvoidUsingCmdletAliases' -Because $assertionMessage } $testFilePath = "TestDrive:\testfile.ps1" Set-Content $testFilePath -value $script It "does not throw and detect one expected warning after the parse error has occured when using -Path parameter set" { $warnings = Invoke-ScriptAnalyzer -Path $testFilePath - $warnings.Count | Should -Be 1 -Because 'PSSA should analyze the whole script after the parse error on [IStorageContext] and find the AvoidAlias warning due to gci' + $warnings.Count | Should -Be 1 -Because $assertionMessage + $warnings.RuleName | Should -Be 'PSAvoidUsingCmdletAliases' -Because $assertionMessage } } } From 057554801063e66b795f8096b458d102609d6bee Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Thu, 5 Apr 2018 08:46:27 +0100 Subject: [PATCH 08/16] Remove unused dotnet-core NuGet feed that caused build failure due to downtime --- NuGet.Config | 1 - 1 file changed, 1 deletion(-) diff --git a/NuGet.Config b/NuGet.Config index d9071f66f..acba4478b 100644 --- a/NuGet.Config +++ b/NuGet.Config @@ -3,7 +3,6 @@ - From 3564b8eb15113235111db8ac16e07da77a6453c9 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Mon, 9 Apr 2018 20:23:30 +0100 Subject: [PATCH 09/16] Fix typo and move string into resources --- Engine/ScriptAnalyzer.cs | 3 +-- Engine/Strings.resx | 3 +++ Tests/Engine/InvokeScriptAnalyzer.tests.ps1 | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 934343075..d59f14fa8 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1663,8 +1663,7 @@ private List RemoveTypeNotFoundParseErrors(ParseError[] parseErrors) } else { - this.outputWriter.WriteVerbose( - $"Ignoring 'TypeNotFound' parse error on type {parseError.Extent}, which is likely due to the type not being knowm due to e.g. 'using' statements. Parse error message was '{parseError.Message}'."); + this.outputWriter.WriteVerbose(string.Format(Strings.TypeNotFoundParseErrorFound, parseError.Extent, parseError.Message)); } } diff --git a/Engine/Strings.resx b/Engine/Strings.resx index 99d90761f..958f3279d 100644 --- a/Engine/Strings.resx +++ b/Engine/Strings.resx @@ -324,4 +324,7 @@ Settings object could not be resolved. + + Ignoring 'TypeNotFound' parse error on type {0}, which can also be due to the type not being known due to e.g. 'using' statements. Parse error message was '{1}'. + \ No newline at end of file diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 index 6fb7165f8..1e28b70e5 100644 --- a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 +++ b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 @@ -558,7 +558,7 @@ Describe "Test -EnableExit Switch" { using namespace Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkModels using namespace Microsoft.Azure.Commands.Common.Authentication.Abstractions Import-Module "AzureRm" - class MyClass { [IStorageContext]$StorageContext } # This will result in a parser error due to [IStorageContext] type that comes from the using stetement but is not known at parse time + class MyClass { [IStorageContext]$StorageContext } # This will result in a parser error due to [IStorageContext] type that comes from the using statement but is not known at parse time gci # Produce AvoidAlias rule '@ $assertionMessage = 'PSSA should analyze the whole script after the parse error on [IStorageContext] and find the PSAvoidUsingCmdletAliases warning due to gci' From 321582e6698296d4deed2078e7f77a5c08e99f18 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Mon, 9 Apr 2018 20:46:08 +0100 Subject: [PATCH 10/16] fix compilation error (forgot to check-in file) --- Engine/Strings.Designer.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Engine/Strings.Designer.cs b/Engine/Strings.Designer.cs index d7234f283..823a17825 100644 --- a/Engine/Strings.Designer.cs +++ b/Engine/Strings.Designer.cs @@ -10,7 +10,6 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer { using System; - using System.Reflection; /// @@ -40,7 +39,7 @@ internal Strings() { internal static global::System.Resources.ResourceManager ResourceManager { get { if (object.ReferenceEquals(resourceMan, null)) { - global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("Microsoft.Windows.PowerShell.ScriptAnalyzer.Strings", typeof(Strings).GetTypeInfo().Assembly); + global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("Microsoft.Windows.PowerShell.ScriptAnalyzer.Strings", typeof(Strings).Assembly); resourceMan = temp; } return resourceMan; @@ -592,6 +591,15 @@ internal static string TextLinesNoNullItem { } } + /// + /// Looks up a localized string similar to Ignoring 'TypeNotFound' parse error on type {0}, which can also be due to the type not being known due to e.g. 'using' statements. Parse error message was '{1}'.. + /// + internal static string TypeNotFoundParseErrorFound { + get { + return ResourceManager.GetString("TypeNotFoundParseErrorFound", resourceCulture); + } + } + /// /// Looks up a localized string similar to Analyzing file: {0}. /// From 259a730e3ad84d22ae7b7141478c32553b90b5b1 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Tue, 10 Apr 2018 08:02:34 +0100 Subject: [PATCH 11/16] fix typo to also build for psv3 --- tools/appveyor.psm1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/appveyor.psm1 b/tools/appveyor.psm1 index fca49b807..0625dea33 100644 --- a/tools/appveyor.psm1 +++ b/tools/appveyor.psm1 @@ -44,7 +44,7 @@ function Invoke-AppVeyorBuild { $BuildType, [Parameter(Mandatory)] - [ValidateSet('Release', 'PSv4Release', 'PSv4Release')] + [ValidateSet('Release', 'PSv3Release', 'PSv4Release')] $BuildConfiguration, [Parameter(Mandatory)] From 897e4268f905f0081db9c036565bb7d4a7619b62 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Tue, 10 Apr 2018 21:19:10 +0100 Subject: [PATCH 12/16] Since the using statement was introduced only inn v5, move to block that runs only on v5+ --- Tests/Engine/InvokeScriptAnalyzer.tests.ps1 | 49 ++++++++++----------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 index 1e28b70e5..15063a629 100644 --- a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 +++ b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 @@ -73,6 +73,30 @@ Describe "Test available parameters" { $params["SaveDscDependency"].ParameterType.FullName | Should -Be "System.Management.Automation.SwitchParameter" } } + + Describe "Handles parse errors due to unknown types" { + $script = @' + using namespace Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkModels + using namespace Microsoft.Azure.Commands.Common.Authentication.Abstractions + Import-Module "AzureRm" + class MyClass { [IStorageContext]$StorageContext } # This will result in a parser error due to [IStorageContext] type that comes from the using statement but is not known at parse time + gci # Produce AvoidAlias rule +'@ + $assertionMessage = 'PSSA should analyze the whole script after the parse error on [IStorageContext] and find the PSAvoidUsingCmdletAliases warning due to gci' + It "does not throw and detect one expected warning after the parse error has occured when using -ScriptDefintion parameter set" { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition $script + $warnings.Count | Should -Be 1 -Because $assertionMessage + $warnings.RuleName | Should -Be 'PSAvoidUsingCmdletAliases' -Because $assertionMessage + } + + $testFilePath = "TestDrive:\testfile.ps1" + Set-Content $testFilePath -value $script + It "does not throw and detect one expected warning after the parse error has occured when using -Path parameter set" { + $warnings = Invoke-ScriptAnalyzer -Path $testFilePath + $warnings.Count | Should -Be 1 -Because $assertionMessage + $warnings.RuleName | Should -Be 'PSAvoidUsingCmdletAliases' -Because $assertionMessage + } + } } Context "It has 2 parameter sets: File and ScriptDefinition" { @@ -552,29 +576,4 @@ Describe "Test -EnableExit Switch" { } } - if (!$testingLibraryUsage) { - Describe "Handles parse errors due to unknown types" { - $script = @' - using namespace Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkModels - using namespace Microsoft.Azure.Commands.Common.Authentication.Abstractions - Import-Module "AzureRm" - class MyClass { [IStorageContext]$StorageContext } # This will result in a parser error due to [IStorageContext] type that comes from the using statement but is not known at parse time - gci # Produce AvoidAlias rule -'@ - $assertionMessage = 'PSSA should analyze the whole script after the parse error on [IStorageContext] and find the PSAvoidUsingCmdletAliases warning due to gci' - It "does not throw and detect one expected warning after the parse error has occured when using -ScriptDefintion parameter set" { - $warnings = Invoke-ScriptAnalyzer -ScriptDefinition $script - $warnings.Count | Should -Be 1 -Because $assertionMessage - $warnings.RuleName | Should -Be 'PSAvoidUsingCmdletAliases' -Because $assertionMessage - } - - $testFilePath = "TestDrive:\testfile.ps1" - Set-Content $testFilePath -value $script - It "does not throw and detect one expected warning after the parse error has occured when using -Path parameter set" { - $warnings = Invoke-ScriptAnalyzer -Path $testFilePath - $warnings.Count | Should -Be 1 -Because $assertionMessage - $warnings.RuleName | Should -Be 'PSAvoidUsingCmdletAliases' -Because $assertionMessage - } - } - } } From 3201b9adaebeca490a90fc24757eb3cbf4b3b25f Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Tue, 10 Apr 2018 21:39:46 +0100 Subject: [PATCH 13/16] fix tests by moving the tests (clearly a smell that something is wrong in this test script --- Tests/Engine/InvokeScriptAnalyzer.tests.ps1 | 50 +++++++++++---------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 index 15063a629..00ba220ed 100644 --- a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 +++ b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 @@ -73,30 +73,6 @@ Describe "Test available parameters" { $params["SaveDscDependency"].ParameterType.FullName | Should -Be "System.Management.Automation.SwitchParameter" } } - - Describe "Handles parse errors due to unknown types" { - $script = @' - using namespace Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkModels - using namespace Microsoft.Azure.Commands.Common.Authentication.Abstractions - Import-Module "AzureRm" - class MyClass { [IStorageContext]$StorageContext } # This will result in a parser error due to [IStorageContext] type that comes from the using statement but is not known at parse time - gci # Produce AvoidAlias rule -'@ - $assertionMessage = 'PSSA should analyze the whole script after the parse error on [IStorageContext] and find the PSAvoidUsingCmdletAliases warning due to gci' - It "does not throw and detect one expected warning after the parse error has occured when using -ScriptDefintion parameter set" { - $warnings = Invoke-ScriptAnalyzer -ScriptDefinition $script - $warnings.Count | Should -Be 1 -Because $assertionMessage - $warnings.RuleName | Should -Be 'PSAvoidUsingCmdletAliases' -Because $assertionMessage - } - - $testFilePath = "TestDrive:\testfile.ps1" - Set-Content $testFilePath -value $script - It "does not throw and detect one expected warning after the parse error has occured when using -Path parameter set" { - $warnings = Invoke-ScriptAnalyzer -Path $testFilePath - $warnings.Count | Should -Be 1 -Because $assertionMessage - $warnings.RuleName | Should -Be 'PSAvoidUsingCmdletAliases' -Because $assertionMessage - } - } } Context "It has 2 parameter sets: File and ScriptDefinition" { @@ -576,4 +552,30 @@ Describe "Test -EnableExit Switch" { } } + # using statements are only supported in v5+ + if (!$testingLibraryUsage -and ($PSVersionTable.PSVersion -ge [Version]'5.0.0')) { + Describe "Handles parse errors due to unknown types" { + $script = @' + using namespace Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkModels + using namespace Microsoft.Azure.Commands.Common.Authentication.Abstractions + Import-Module "AzureRm" + class MyClass { [IStorageContext]$StorageContext } # This will result in a parser error due to [IStorageContext] type that comes from the using statement but is not known at parse time + gci # Produce AvoidAlias rule +'@ + $assertionMessage = 'PSSA should analyze the whole script after the parse error on [IStorageContext] and find the PSAvoidUsingCmdletAliases warning due to gci' + It "does not throw and detect one expected warning after the parse error has occured when using -ScriptDefintion parameter set" { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition $script + $warnings.Count | Should -Be 1 -Because $assertionMessage + $warnings.RuleName | Should -Be 'PSAvoidUsingCmdletAliases' -Because $assertionMessage + } + + $testFilePath = "TestDrive:\testfile.ps1" + Set-Content $testFilePath -value $script + It "does not throw and detect one expected warning after the parse error has occured when using -Path parameter set" { + $warnings = Invoke-ScriptAnalyzer -Path $testFilePath + $warnings.Count | Should -Be 1 -Because $assertionMessage + $warnings.RuleName | Should -Be 'PSAvoidUsingCmdletAliases' -Because $assertionMessage + } + } + } } From 9792f96b614b95f0a6a455beb5a1b284d8e144e0 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Tue, 10 Apr 2018 21:49:00 +0100 Subject: [PATCH 14/16] write warning on parse errors instead of verbose output and tweak message --- Engine/ScriptAnalyzer.cs | 2 +- Engine/Strings.Designer.cs | 2 +- Engine/Strings.resx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index d59f14fa8..502cd3f35 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1663,7 +1663,7 @@ private List RemoveTypeNotFoundParseErrors(ParseError[] parseErrors) } else { - this.outputWriter.WriteVerbose(string.Format(Strings.TypeNotFoundParseErrorFound, parseError.Extent, parseError.Message)); + this.outputWriter.WriteWarning(string.Format(Strings.TypeNotFoundParseErrorFound, parseError.Extent, parseError.Message)); } } diff --git a/Engine/Strings.Designer.cs b/Engine/Strings.Designer.cs index 823a17825..27e0bf247 100644 --- a/Engine/Strings.Designer.cs +++ b/Engine/Strings.Designer.cs @@ -592,7 +592,7 @@ internal static string TextLinesNoNullItem { } /// - /// Looks up a localized string similar to Ignoring 'TypeNotFound' parse error on type {0}, which can also be due to the type not being known due to e.g. 'using' statements. Parse error message was '{1}'.. + /// Looks up a localized string similar to Ignoring 'TypeNotFound' parse error on type {0}, which can be due to the type not being known due to e.g. 'using' statements. Parse error message was '{1}'.. /// internal static string TypeNotFoundParseErrorFound { get { diff --git a/Engine/Strings.resx b/Engine/Strings.resx index 958f3279d..eb48b1a77 100644 --- a/Engine/Strings.resx +++ b/Engine/Strings.resx @@ -325,6 +325,6 @@ Settings object could not be resolved. - Ignoring 'TypeNotFound' parse error on type {0}, which can also be due to the type not being known due to e.g. 'using' statements. Parse error message was '{1}'. + Ignoring 'TypeNotFound' parse error on type {0}, which can be due to the type not being known due to e.g. 'using' statements. Parse error message was '{1}'. \ No newline at end of file From 9636d4347795b4b9456f1526be9aa71913a902f4 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Wed, 11 Apr 2018 22:14:13 +0100 Subject: [PATCH 15/16] instead of returning a warning, return a diagnostic record with a custom rule name and tweak error message --- Engine/ScriptAnalyzer.cs | 18 +++++++++++------- Engine/Strings.Designer.cs | 2 +- Engine/Strings.resx | 2 +- Tests/Engine/InvokeScriptAnalyzer.tests.ps1 | 10 ++++------ 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 502cd3f35..6176137fd 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1522,7 +1522,7 @@ public IEnumerable AnalyzeScriptDefinition(string scriptDefini return null; } - var relevantParseErrors = RemoveTypeNotFoundParseErrors(errors); + var relevantParseErrors = RemoveTypeNotFoundParseErrors(errors, out List diagnosticRecords); if (relevantParseErrors != null && relevantParseErrors.Count > 0) { @@ -1541,7 +1541,7 @@ public IEnumerable AnalyzeScriptDefinition(string scriptDefini return new List(); } - return this.AnalyzeSyntaxTree(scriptAst, scriptTokens, String.Empty); + return diagnosticRecords.Concat(this.AnalyzeSyntaxTree(scriptAst, scriptTokens, String.Empty)); } /// @@ -1651,10 +1651,12 @@ private static Encoding GetFileEncoding(string path) /// /// /// List of relevant parse errors. - private List RemoveTypeNotFoundParseErrors(ParseError[] parseErrors) + private List RemoveTypeNotFoundParseErrors(ParseError[] parseErrors, out List diagnosticRecords) { var relevantParseErrors = new List(); - foreach(var parseError in parseErrors) + diagnosticRecords = new List(); + + foreach (var parseError in parseErrors) { // If types are not known due them not being imported yet, the parser throws an error that can be ignored if (parseError.ErrorId != "TypeNotFound") @@ -1663,7 +1665,8 @@ private List RemoveTypeNotFoundParseErrors(ParseError[] parseErrors) } else { - this.outputWriter.WriteWarning(string.Format(Strings.TypeNotFoundParseErrorFound, parseError.Extent, parseError.Message)); + diagnosticRecords.Add(new DiagnosticRecord( + string.Format(Strings.TypeNotFoundParseErrorFound, parseError.Extent), parseError.Extent, "TypeNotFound", DiagnosticSeverity.Information, parseError.Extent.File)); } } @@ -1832,6 +1835,7 @@ private IEnumerable AnalyzeFile(string filePath) ParseError[] errors = null; this.outputWriter.WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseFileMessage, filePath)); + var diagnosticRecords = new List(); //Parse the file if (File.Exists(filePath)) @@ -1855,7 +1859,7 @@ private IEnumerable AnalyzeFile(string filePath) scriptAst = Parser.ParseFile(filePath, out scriptTokens, out errors); } #endif //!PSV3 - var relevantParseErrors = RemoveTypeNotFoundParseErrors(errors); + var relevantParseErrors = RemoveTypeNotFoundParseErrors(errors, out diagnosticRecords); //Runspace.DefaultRunspace = oldDefault; if (relevantParseErrors != null && relevantParseErrors.Count > 0) @@ -1884,7 +1888,7 @@ private IEnumerable AnalyzeFile(string filePath) return null; } - return this.AnalyzeSyntaxTree(scriptAst, scriptTokens, filePath); + return diagnosticRecords.Concat(this.AnalyzeSyntaxTree(scriptAst, scriptTokens, filePath)); } private bool IsModuleNotFoundError(ParseError error) diff --git a/Engine/Strings.Designer.cs b/Engine/Strings.Designer.cs index 27e0bf247..077b3a591 100644 --- a/Engine/Strings.Designer.cs +++ b/Engine/Strings.Designer.cs @@ -592,7 +592,7 @@ internal static string TextLinesNoNullItem { } /// - /// Looks up a localized string similar to Ignoring 'TypeNotFound' parse error on type {0}, which can be due to the type not being known due to e.g. 'using' statements. Parse error message was '{1}'.. + /// Looks up a localized string similar to Ignoring 'TypeNotFound' parse error on type '{0}'. Check if the specified type is correct. This can also be due the type not being known at parse time due to types imported by 'using' statements.. /// internal static string TypeNotFoundParseErrorFound { get { diff --git a/Engine/Strings.resx b/Engine/Strings.resx index eb48b1a77..743c3ccad 100644 --- a/Engine/Strings.resx +++ b/Engine/Strings.resx @@ -325,6 +325,6 @@ Settings object could not be resolved. - Ignoring 'TypeNotFound' parse error on type {0}, which can be due to the type not being known due to e.g. 'using' statements. Parse error message was '{1}'. + Ignoring 'TypeNotFound' parse error on type '{0}'. Check if the specified type is correct. This can also be due the type not being known at parse time due to types imported by 'using' statements. \ No newline at end of file diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 index 00ba220ed..049b0f682 100644 --- a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 +++ b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 @@ -560,21 +560,19 @@ Describe "Test -EnableExit Switch" { using namespace Microsoft.Azure.Commands.Common.Authentication.Abstractions Import-Module "AzureRm" class MyClass { [IStorageContext]$StorageContext } # This will result in a parser error due to [IStorageContext] type that comes from the using statement but is not known at parse time - gci # Produce AvoidAlias rule '@ - $assertionMessage = 'PSSA should analyze the whole script after the parse error on [IStorageContext] and find the PSAvoidUsingCmdletAliases warning due to gci' It "does not throw and detect one expected warning after the parse error has occured when using -ScriptDefintion parameter set" { $warnings = Invoke-ScriptAnalyzer -ScriptDefinition $script - $warnings.Count | Should -Be 1 -Because $assertionMessage - $warnings.RuleName | Should -Be 'PSAvoidUsingCmdletAliases' -Because $assertionMessage + $warnings.Count | Should -Be 1 + $warnings.RuleName | Should -Be 'TypeNotFound' } $testFilePath = "TestDrive:\testfile.ps1" Set-Content $testFilePath -value $script It "does not throw and detect one expected warning after the parse error has occured when using -Path parameter set" { $warnings = Invoke-ScriptAnalyzer -Path $testFilePath - $warnings.Count | Should -Be 1 -Because $assertionMessage - $warnings.RuleName | Should -Be 'PSAvoidUsingCmdletAliases' -Because $assertionMessage + $warnings.Count | Should -Be 1 + $warnings.RuleName | Should -Be 'TypeNotFound' } } } From 2b4473277ffaaeca2ff874ccbb977cd5f8f00c35 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Wed, 11 Apr 2018 22:26:38 +0100 Subject: [PATCH 16/16] poke build due to sporadic AppVeyor failure on setup (not a test) -> rename variable --- Engine/ScriptAnalyzer.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 6176137fd..1c6a64fff 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1526,10 +1526,10 @@ public IEnumerable AnalyzeScriptDefinition(string scriptDefini if (relevantParseErrors != null && relevantParseErrors.Count > 0) { - foreach (ParseError error in relevantParseErrors) + foreach (var parseError in relevantParseErrors) { - string parseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParseErrorFormatForScriptDefinition, error.Message.TrimEnd('.'), error.Extent.StartLineNumber, error.Extent.StartColumnNumber); - this.outputWriter.WriteError(new ErrorRecord(new ParseException(parseErrorMessage), parseErrorMessage, ErrorCategory.ParserError, error.ErrorId)); + string parseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParseErrorFormatForScriptDefinition, parseError.Message.TrimEnd('.'), parseError.Extent.StartLineNumber, parseError.Extent.StartColumnNumber); + this.outputWriter.WriteError(new ErrorRecord(new ParseException(parseErrorMessage), parseErrorMessage, ErrorCategory.ParserError, parseError.ErrorId)); } } @@ -1864,10 +1864,10 @@ private IEnumerable AnalyzeFile(string filePath) //Runspace.DefaultRunspace = oldDefault; if (relevantParseErrors != null && relevantParseErrors.Count > 0) { - foreach (ParseError error in relevantParseErrors) + foreach (var parseError in relevantParseErrors) { - string parseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParserErrorFormat, error.Extent.File, error.Message.TrimEnd('.'), error.Extent.StartLineNumber, error.Extent.StartColumnNumber); - this.outputWriter.WriteError(new ErrorRecord(new ParseException(parseErrorMessage), parseErrorMessage, ErrorCategory.ParserError, error.ErrorId)); + string parseErrorMessage = String.Format(CultureInfo.CurrentCulture, Strings.ParserErrorFormat, parseError.Extent.File, parseError.Message.TrimEnd('.'), parseError.Extent.StartLineNumber, parseError.Extent.StartColumnNumber); + this.outputWriter.WriteError(new ErrorRecord(new ParseException(parseErrorMessage), parseErrorMessage, ErrorCategory.ParserError, parseError.ErrorId)); } }