From 1da47c4204d73f1c22904d5a5426b3b3e351d6cd Mon Sep 17 00:00:00 2001 From: iRon7 Date: Wed, 22 Apr 2026 18:38:41 +0200 Subject: [PATCH 01/10] #2098 Add new MissingTryBlock rule --- Rules/MissingTryBlock.cs | 83 ++++++++++++++++ Rules/Strings.resx | 12 +++ Tests/Rules/MissingTryBlock.tests.ps1 | 133 ++++++++++++++++++++++++++ docs/Rules/MissingTryBlock.md | 34 +++++++ docs/Rules/README.md | 1 + 5 files changed, 263 insertions(+) create mode 100644 Rules/MissingTryBlock.cs create mode 100644 Tests/Rules/MissingTryBlock.tests.ps1 create mode 100644 docs/Rules/MissingTryBlock.md diff --git a/Rules/MissingTryBlock.cs b/Rules/MissingTryBlock.cs new file mode 100644 index 000000000..0ef2a0d23 --- /dev/null +++ b/Rules/MissingTryBlock.cs @@ -0,0 +1,83 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Management.Automation.Language; + +#if !CORECLR +using System.ComponentModel.Composition; +#endif + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules +{ +#if !CORECLR + [Export(typeof(IScriptRule))] +#endif + + /// + /// Rule that warns when reserved words are used as function names + /// + public class MissingTryBlock : IScriptRule + { + /// + /// Analyzes the PowerShell AST for uses of reserved words as function names. + /// + /// The PowerShell Abstract Syntax Tree to analyze. + /// The name of the file being analyzed (for diagnostic reporting). + /// A collection of diagnostic records for each violation. + public IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); + + // Find all FunctionDefinitionAst in the Ast + var missingTryAsts = ast.FindAll(testAst => + // Normally should be part of a TryStatementAst + testAst is StringConstantExpressionAst stringAst && + // Catch of finally are reserved keywords and should be bare words + stringAst.StringConstantType == StringConstantType.BareWord && + ( + String.Equals(stringAst.Value, "catch", StringComparison.OrdinalIgnoreCase) || + String.Equals(stringAst.Value, "finally", StringComparison.OrdinalIgnoreCase) + ) && + stringAst.Parent is CommandAst commandAst && + // Only violate if the catch or finally is the first command element + commandAst.CommandElements[0] == stringAst, + true + ); + + foreach (StringConstantExpressionAst missingTryAst in missingTryAsts) + { + yield return new DiagnosticRecord( + string.Format( + CultureInfo.CurrentCulture, + Strings.MissingTryBlockError, + CultureInfo.CurrentCulture.TextInfo.ToTitleCase(missingTryAst.Value)), + missingTryAst.Extent, + GetName(), + DiagnosticSeverity.Error, + fileName, + missingTryAst.Value + ); + } + } + + public string GetCommonName() => Strings.MissingTryBlockCommonName; + + public string GetDescription() => Strings.MissingTryBlockDescription; + + public string GetName() => string.Format( + CultureInfo.CurrentCulture, + Strings.NameSpaceFormat, + GetSourceName(), + Strings.MissingTryBlockName); + + public RuleSeverity GetSeverity() => RuleSeverity.Warning; + + public string GetSourceName() => Strings.SourceName; + + public SourceType GetSourceType() => SourceType.Builtin; + } +} \ No newline at end of file diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 2a04fd759..1b868ea90 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -276,6 +276,18 @@ Module Manifest Fields + + MissingTryBlock + + + Missing try block + + + The catch and finally blocks should be preceded by a try block. + + + {0} is missing a try block + If a script file is in a PowerShell module folder, then that folder must be loadable. diff --git a/Tests/Rules/MissingTryBlock.tests.ps1 b/Tests/Rules/MissingTryBlock.tests.ps1 new file mode 100644 index 000000000..bb28ced9b --- /dev/null +++ b/Tests/Rules/MissingTryBlock.tests.ps1 @@ -0,0 +1,133 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +[Diagnostics.CodeAnalysis.SuppressMessage('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'False positive')] +param() + +BeforeAll { + $ruleName = "PSMissingTryBlock" +} + +Describe "MissingTryBlock" { + Context "Violates" { + It "Catch is missing a try block" { + $scriptDefinition = { catch { "An error occurred." } }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Error + $violations.Extent.Text | Should -Be catch + $violations.Message | Should -Be 'Catch is missing a try block' + $violations.RuleSuppressionID | Should -Be catch + } + + It "Finally is missing a try block" { + $scriptDefinition = { finally { "Finalizing..." } }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Error + $violations.Extent.Text | Should -Be finally + $violations.Message | Should -Be 'Finally is missing a try block' + $violations.RuleSuppressionID | Should -Be finally + } + + It "Single line catch and finally is missing a try block" { + $scriptDefinition = { + catch { "An error occurred." } finally { "Finalizing..." } + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Error + $violations.Extent.Text | Should -Be catch + $violations.Message | Should -Be 'Catch is missing a try block' + $violations.RuleSuppressionID | Should -Be catch + } + + It "Multi line catch and finally is missing a try block" { + $scriptDefinition = { + catch { "An error occurred." } + finally { "Finalizing..." } + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 2 + $violations[0].Severity | Should -Be Error + $violations[0].Extent.Text | Should -Be catch + $violations[0].Message | Should -Be 'Catch is missing a try block' + $violations[0].RuleSuppressionID | Should -Be catch + $violations[1].Severity | Should -Be Error + $violations[1].Extent.Text | Should -Be finally + $violations[1].Message | Should -Be 'Finally is missing a try block' + $violations[1].RuleSuppressionID | Should -Be finally + } + } + + Context "Compliant" { + It "try-catch block" { + $scriptDefinition = { + try { NonsenseString } + catch { "An error occurred." } + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + + It "try-catch-final statement" { + $scriptDefinition = { + try { NonsenseString } + catch { "An error occurred." } + finally { "Finalizing..." } + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + + It "Single line try statement" { + $scriptDefinition = { + try { NonsenseString } catch { "An error occurred." } finally { "Finalizing..." } + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + + It "Catch as parameter" { + $scriptDefinition = { Write-Host Catch }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + + It "Catch as double quoted string" { + $scriptDefinition = { "Catch" }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + + It "Catch as single quoted string" { + $scriptDefinition = { 'Catch' }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + } + + Context "Suppressed" { + It "Multi line catch and finally is missing a try block" { + $scriptDefinition = { + [Diagnostics.CodeAnalysis.SuppressMessage('PSMissingTryBlock', '', Justification = 'Test')] + param() + catch { "An error occurred." } + finally { "Finalizing..." } + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations | Should -BeNullOrEmpty + } + + It "Multi line catch and finally is missing a try block for catch only" { + $scriptDefinition = { + [Diagnostics.CodeAnalysis.SuppressMessage('PSMissingTryBlock', 'finally', Justification = 'Test')] + param() + catch { "An error occurred." } + finally { "Finalizing..." } + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + } + } +} \ No newline at end of file diff --git a/docs/Rules/MissingTryBlock.md b/docs/Rules/MissingTryBlock.md new file mode 100644 index 000000000..eb18a5f65 --- /dev/null +++ b/docs/Rules/MissingTryBlock.md @@ -0,0 +1,34 @@ +--- +description: Missing Try Block +ms.date: 04/22/2026 +ms.topic: reference +title: MissingTryBlock +--- +# MissingTryBlock + +**Severity Level: Error** + +## Description + +The `catch` and `finally` blocks should be preceded by a `try` block. +Otherwise, the `catch` and `finally` blocks will be interpreted as commands, which is likely a mistake and result +in a "*The term 'catch' is not recognized as a name of a cmdlet*" error at runtime. + +## How + +Add a `try` block before the `catch` and `finally` blocks. + +## Example + +### Wrong + +```powershell +catch { "An error occurred." } +``` + +### Correct + +```powershell +try { $a = 1 / $b } +catch { "Attempted to divide by zero." } +``` diff --git a/docs/Rules/README.md b/docs/Rules/README.md index fca031e33..846eb0799 100644 --- a/docs/Rules/README.md +++ b/docs/Rules/README.md @@ -50,6 +50,7 @@ The PSScriptAnalyzer contains the following rule definitions. | [DSCUseVerboseMessageInDSCResource](./DSCUseVerboseMessageInDSCResource.md) | Error | Yes | | | [MisleadingBacktick](./MisleadingBacktick.md) | Warning | Yes | | | [MissingModuleManifestField](./MissingModuleManifestField.md) | Warning | Yes | | +| [MissingTryBlock](./MissingTryBlock.md) | Error | Yes | | | [PlaceCloseBrace](./PlaceCloseBrace.md) | Warning | No | Yes | | [PlaceOpenBrace](./PlaceOpenBrace.md) | Warning | No | Yes | | [PossibleIncorrectComparisonWithNull](./PossibleIncorrectComparisonWithNull.md) | Warning | Yes | | From e9a23659092dd342ac9f13df050a519ce9681fce Mon Sep 17 00:00:00 2001 From: iRon7 Date: Wed, 29 Apr 2026 16:48:56 +0200 Subject: [PATCH 02/10] Changed severity level of MissingTryBlock rule from Error to Warning in the rule definition and resolved copy-pasta in the rule description. Co-authored-by: Copilot --- Rules/MissingTryBlock.cs | 6 +++--- docs/Rules/MissingTryBlock.md | 2 +- docs/Rules/README.md | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Rules/MissingTryBlock.cs b/Rules/MissingTryBlock.cs index 0ef2a0d23..a123682ac 100644 --- a/Rules/MissingTryBlock.cs +++ b/Rules/MissingTryBlock.cs @@ -18,12 +18,12 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules #endif /// - /// Rule that warns when reserved words are used as function names + /// Rule that warns when catch or finally blocks are used without a corresponding try block /// public class MissingTryBlock : IScriptRule { /// - /// Analyzes the PowerShell AST for uses of reserved words as function names. + /// Analyzes the PowerShell AST for catch - or finally blocks that misses the try block. /// /// The PowerShell Abstract Syntax Tree to analyze. /// The name of the file being analyzed (for diagnostic reporting). @@ -57,7 +57,7 @@ stringAst.Parent is CommandAst commandAst && CultureInfo.CurrentCulture.TextInfo.ToTitleCase(missingTryAst.Value)), missingTryAst.Extent, GetName(), - DiagnosticSeverity.Error, + DiagnosticSeverity.Warning, fileName, missingTryAst.Value ); diff --git a/docs/Rules/MissingTryBlock.md b/docs/Rules/MissingTryBlock.md index eb18a5f65..4dd5f40e2 100644 --- a/docs/Rules/MissingTryBlock.md +++ b/docs/Rules/MissingTryBlock.md @@ -6,7 +6,7 @@ title: MissingTryBlock --- # MissingTryBlock -**Severity Level: Error** +**Severity Level: Warning** ## Description diff --git a/docs/Rules/README.md b/docs/Rules/README.md index 846eb0799..41e7b8916 100644 --- a/docs/Rules/README.md +++ b/docs/Rules/README.md @@ -50,7 +50,7 @@ The PSScriptAnalyzer contains the following rule definitions. | [DSCUseVerboseMessageInDSCResource](./DSCUseVerboseMessageInDSCResource.md) | Error | Yes | | | [MisleadingBacktick](./MisleadingBacktick.md) | Warning | Yes | | | [MissingModuleManifestField](./MissingModuleManifestField.md) | Warning | Yes | | -| [MissingTryBlock](./MissingTryBlock.md) | Error | Yes | | +| [MissingTryBlock](./MissingTryBlock.md) | Warning | Yes | | | [PlaceCloseBrace](./PlaceCloseBrace.md) | Warning | No | Yes | | [PlaceOpenBrace](./PlaceOpenBrace.md) | Warning | No | Yes | | [PossibleIncorrectComparisonWithNull](./PossibleIncorrectComparisonWithNull.md) | Warning | Yes | | From 50f751fd1577f30e7abcd4f309fd1b5f42e67e3c Mon Sep 17 00:00:00 2001 From: iRon7 Date: Wed, 29 Apr 2026 16:55:23 +0200 Subject: [PATCH 03/10] Updated tests for MissingTryBlock rule to reflect severity change from Error to Warning. --- Tests/Rules/MissingTryBlock.tests.ps1 | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Tests/Rules/MissingTryBlock.tests.ps1 b/Tests/Rules/MissingTryBlock.tests.ps1 index bb28ced9b..56a7094c6 100644 --- a/Tests/Rules/MissingTryBlock.tests.ps1 +++ b/Tests/Rules/MissingTryBlock.tests.ps1 @@ -14,7 +14,7 @@ Describe "MissingTryBlock" { $scriptDefinition = { catch { "An error occurred." } }.ToString() $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) $violations.Count | Should -Be 1 - $violations.Severity | Should -Be Error + $violations.Severity | Should -Be Warning $violations.Extent.Text | Should -Be catch $violations.Message | Should -Be 'Catch is missing a try block' $violations.RuleSuppressionID | Should -Be catch @@ -24,7 +24,7 @@ Describe "MissingTryBlock" { $scriptDefinition = { finally { "Finalizing..." } }.ToString() $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) $violations.Count | Should -Be 1 - $violations.Severity | Should -Be Error + $violations.Severity | Should -Be Warning $violations.Extent.Text | Should -Be finally $violations.Message | Should -Be 'Finally is missing a try block' $violations.RuleSuppressionID | Should -Be finally @@ -36,7 +36,7 @@ Describe "MissingTryBlock" { }.ToString() $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) $violations.Count | Should -Be 1 - $violations.Severity | Should -Be Error + $violations.Severity | Should -Be Warning $violations.Extent.Text | Should -Be catch $violations.Message | Should -Be 'Catch is missing a try block' $violations.RuleSuppressionID | Should -Be catch @@ -49,11 +49,11 @@ Describe "MissingTryBlock" { }.ToString() $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) $violations.Count | Should -Be 2 - $violations[0].Severity | Should -Be Error + $violations[0].Severity | Should -Be Warning $violations[0].Extent.Text | Should -Be catch $violations[0].Message | Should -Be 'Catch is missing a try block' $violations[0].RuleSuppressionID | Should -Be catch - $violations[1].Severity | Should -Be Error + $violations[1].Severity | Should -Be Warning $violations[1].Extent.Text | Should -Be finally $violations[1].Message | Should -Be 'Finally is missing a try block' $violations[1].RuleSuppressionID | Should -Be finally From 79e80aa4ce93a553714e431a98baac4bb9e26a7b Mon Sep 17 00:00:00 2001 From: iRon7 Date: Wed, 29 Apr 2026 17:37:50 +0200 Subject: [PATCH 04/10] Update Rules/MissingTryBlock.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- Rules/MissingTryBlock.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rules/MissingTryBlock.cs b/Rules/MissingTryBlock.cs index a123682ac..8def80fc0 100644 --- a/Rules/MissingTryBlock.cs +++ b/Rules/MissingTryBlock.cs @@ -23,7 +23,7 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules public class MissingTryBlock : IScriptRule { /// - /// Analyzes the PowerShell AST for catch - or finally blocks that misses the try block. + /// Analyzes the PowerShell AST for catch or finally blocks that miss the try block. /// /// The PowerShell Abstract Syntax Tree to analyze. /// The name of the file being analyzed (for diagnostic reporting). From 47507bbc695525bf238b7e174e71727258daaeac Mon Sep 17 00:00:00 2001 From: iRon7 Date: Wed, 29 Apr 2026 18:00:26 +0200 Subject: [PATCH 05/10] =?UTF-8?q?Grammar:=20=E2=80=9Cwhich=20is=20likely?= =?UTF-8?q?=20a=20mistake=20and=20result=20in=20=E2=80=A6=20error=E2=80=9D?= =?UTF-8?q?=20is=20ungrammatical.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/Rules/MissingTryBlock.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/Rules/MissingTryBlock.md b/docs/Rules/MissingTryBlock.md index 4dd5f40e2..76a1363ee 100644 --- a/docs/Rules/MissingTryBlock.md +++ b/docs/Rules/MissingTryBlock.md @@ -11,8 +11,8 @@ title: MissingTryBlock ## Description The `catch` and `finally` blocks should be preceded by a `try` block. -Otherwise, the `catch` and `finally` blocks will be interpreted as commands, which is likely a mistake and result -in a "*The term 'catch' is not recognized as a name of a cmdlet*" error at runtime. +Otherwise, the `catch` and `finally` blocks will be interpreted as commands, which is likely a mistake and +will result in a "*The term 'catch' is not recognized as a name of a cmdlet*" error at runtime. ## How From e1a8d3f5ba6ef404f9993b21961a893c198a07f2 Mon Sep 17 00:00:00 2001 From: iRon7 Date: Wed, 29 Apr 2026 18:05:15 +0200 Subject: [PATCH 06/10] Update Rules/Strings.resx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- Rules/Strings.resx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 1b868ea90..a296b04f3 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -280,7 +280,7 @@ MissingTryBlock - Missing try block + Missing Try Block The catch and finally blocks should be preceded by a try block. From a42082080af70731afc9286ce3c6f8c16aea9899 Mon Sep 17 00:00:00 2001 From: iRon7 Date: Wed, 29 Apr 2026 18:08:57 +0200 Subject: [PATCH 07/10] Update Rules/MissingTryBlock.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- Rules/MissingTryBlock.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rules/MissingTryBlock.cs b/Rules/MissingTryBlock.cs index 8def80fc0..9b0b9a96e 100644 --- a/Rules/MissingTryBlock.cs +++ b/Rules/MissingTryBlock.cs @@ -36,7 +36,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) var missingTryAsts = ast.FindAll(testAst => // Normally should be part of a TryStatementAst testAst is StringConstantExpressionAst stringAst && - // Catch of finally are reserved keywords and should be bare words + // Catch or finally are reserved keywords and should be bare words stringAst.StringConstantType == StringConstantType.BareWord && ( String.Equals(stringAst.Value, "catch", StringComparison.OrdinalIgnoreCase) || From 216645d8f716a858e3fd293a91690d222a997042 Mon Sep 17 00:00:00 2001 From: iRon7 Date: Wed, 29 Apr 2026 18:25:00 +0200 Subject: [PATCH 08/10] minor but worthwhile --- Rules/MissingTryBlock.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Rules/MissingTryBlock.cs b/Rules/MissingTryBlock.cs index 9b0b9a96e..1ae2739a6 100644 --- a/Rules/MissingTryBlock.cs +++ b/Rules/MissingTryBlock.cs @@ -32,11 +32,11 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); - // Find all FunctionDefinitionAst in the Ast + // Find the bare word 'catch' or 'finally' StringConstantExpressionAst nodes used as commands var missingTryAsts = ast.FindAll(testAst => // Normally should be part of a TryStatementAst testAst is StringConstantExpressionAst stringAst && - // Catch or finally are reserved keywords and should be bare words + // Check whether "catch" or "finally" are bare words stringAst.StringConstantType == StringConstantType.BareWord && ( String.Equals(stringAst.Value, "catch", StringComparison.OrdinalIgnoreCase) || From f53cc38d58477ba72cfdbb021d8b1f9083e56145 Mon Sep 17 00:00:00 2001 From: iRon7 Date: Thu, 7 May 2026 15:48:03 +0200 Subject: [PATCH 09/10] Update docs/Rules/MissingTryBlock.md Co-authored-by: Sean Wheeler --- docs/Rules/MissingTryBlock.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/docs/Rules/MissingTryBlock.md b/docs/Rules/MissingTryBlock.md index 76a1363ee..2d4bc8831 100644 --- a/docs/Rules/MissingTryBlock.md +++ b/docs/Rules/MissingTryBlock.md @@ -10,9 +10,13 @@ title: MissingTryBlock ## Description -The `catch` and `finally` blocks should be preceded by a `try` block. -Otherwise, the `catch` and `finally` blocks will be interpreted as commands, which is likely a mistake and -will result in a "*The term 'catch' is not recognized as a name of a cmdlet*" error at runtime. +The `catch` and `finally` blocks must be preceded by a `try` block. Without a `try` block, the +`catch` and `finally` are interpreted as commands and result in a runtime error, such as: + +> "The term 'catch' is not recognized as a name of a cmdlet" + +This rule identifies instances where `catch` or `finally` blocks are present with out an associated +`try` block. ## How From 4b94c24aad25e20d63f0a117eb022f30075c1318 Mon Sep 17 00:00:00 2001 From: iRon7 Date: Tue, 12 May 2026 16:40:08 +0200 Subject: [PATCH 10/10] Applied feedback from @andrewconnell to add a note about the rule not being enabled by default, and to add a note about potential false positives with functions named "catch" or "finally". Also added a test context for when the rule is disabled. Updated the rule implementation to inherit from ConfigurableRule and set Enable to false in the constructor. Updated the AnalyzeScript method to be an override, and added overrides for GetCommonName, GetDescription, GetName, GetSeverity, and GetSourceName. --- Rules/MissingTryBlock.cs | 91 ++++++++++++++++++++++----- Tests/Rules/MissingTryBlock.tests.ps1 | 49 +++++++++++---- docs/Rules/MissingTryBlock.md | 27 ++++++++ docs/Rules/README.md | 2 +- 4 files changed, 139 insertions(+), 30 deletions(-) diff --git a/Rules/MissingTryBlock.cs b/Rules/MissingTryBlock.cs index 1ae2739a6..dd48149ae 100644 --- a/Rules/MissingTryBlock.cs +++ b/Rules/MissingTryBlock.cs @@ -1,18 +1,20 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; using System; using System.Collections.Generic; -using System.Globalization; -using System.Management.Automation.Language; - #if !CORECLR using System.ComponentModel.Composition; #endif +using System.Globalization; +using System.Management.Automation.Language; +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules { + /// + /// Find bare word "catch" or "finally" tokens that are not part of a TryStatementAst + /// #if !CORECLR [Export(typeof(IScriptRule))] #endif @@ -20,15 +22,24 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules /// /// Rule that warns when catch or finally blocks are used without a corresponding try block /// - public class MissingTryBlock : IScriptRule + + public class MissingTryBlock : ConfigurableRule { + /// - /// Analyzes the PowerShell AST for catch or finally blocks that miss the try block. + /// Construct an object of MissingTryBlock type. /// - /// The PowerShell Abstract Syntax Tree to analyze. - /// The name of the file being analyzed (for diagnostic reporting). - /// A collection of diagnostic records for each violation. - public IEnumerable AnalyzeScript(Ast ast, string fileName) + public MissingTryBlock() { + Enable = false; + } + + /// + /// Find bare word "catch" or "finally" tokens that are not part of a TryStatementAst + /// + /// AST to be analyzed. This should be non-null + /// Name of file that corresponds to the input AST. + /// A an enumerable type containing the violations + public override IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); @@ -64,20 +75,66 @@ stringAst.Parent is CommandAst commandAst && } } - public string GetCommonName() => Strings.MissingTryBlockCommonName; + /// + /// Retrieves the common name of this rule. + /// + public override string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.MissingTryBlockCommonName); + } - public string GetDescription() => Strings.MissingTryBlockDescription; + /// + /// Retrieves the description of this rule. + /// + public override string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.MissingTryBlockDescription); + } - public string GetName() => string.Format( + /// + /// Retrieves the name of this rule. + /// + public override string GetName() + { + return string.Format( CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.MissingTryBlockName); + } - public RuleSeverity GetSeverity() => RuleSeverity.Warning; + /// + /// Retrieves the severity of the rule: error, warning or information. + /// + public override RuleSeverity GetSeverity() + { + return RuleSeverity.Warning; + } + + /// + /// Gets the severity of the returned diagnostic record: error, warning, or information. + /// + /// + public DiagnosticSeverity GetDiagnosticSeverity() + { + return DiagnosticSeverity.Warning; + } - public string GetSourceName() => Strings.SourceName; + /// + /// Retrieves the name of the module/assembly the rule is from. + /// + public override string GetSourceName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + } - public SourceType GetSourceType() => SourceType.Builtin; + /// + /// Retrieves the type of the rule, Builtin, Managed or Module. + /// + public override SourceType GetSourceType() + { + return SourceType.Builtin; + } } -} \ No newline at end of file +} + diff --git a/Tests/Rules/MissingTryBlock.tests.ps1 b/Tests/Rules/MissingTryBlock.tests.ps1 index 56a7094c6..7a06e4d04 100644 --- a/Tests/Rules/MissingTryBlock.tests.ps1 +++ b/Tests/Rules/MissingTryBlock.tests.ps1 @@ -9,10 +9,18 @@ BeforeAll { } Describe "MissingTryBlock" { + + BeforeAll { + $Settings = @{ + IncludeRules = @($ruleName) + Rules = @{ $ruleName = @{ Enable = $true } } + } + } + Context "Violates" { It "Catch is missing a try block" { $scriptDefinition = { catch { "An error occurred." } }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations.Count | Should -Be 1 $violations.Severity | Should -Be Warning $violations.Extent.Text | Should -Be catch @@ -22,7 +30,7 @@ Describe "MissingTryBlock" { It "Finally is missing a try block" { $scriptDefinition = { finally { "Finalizing..." } }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations.Count | Should -Be 1 $violations.Severity | Should -Be Warning $violations.Extent.Text | Should -Be finally @@ -34,7 +42,7 @@ Describe "MissingTryBlock" { $scriptDefinition = { catch { "An error occurred." } finally { "Finalizing..." } }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations.Count | Should -Be 1 $violations.Severity | Should -Be Warning $violations.Extent.Text | Should -Be catch @@ -47,7 +55,7 @@ Describe "MissingTryBlock" { catch { "An error occurred." } finally { "Finalizing..." } }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations.Count | Should -Be 2 $violations[0].Severity | Should -Be Warning $violations[0].Extent.Text | Should -Be catch @@ -66,7 +74,7 @@ Describe "MissingTryBlock" { try { NonsenseString } catch { "An error occurred." } }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations | Should -BeNullOrEmpty } @@ -76,7 +84,7 @@ Describe "MissingTryBlock" { catch { "An error occurred." } finally { "Finalizing..." } }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations | Should -BeNullOrEmpty } @@ -84,25 +92,25 @@ Describe "MissingTryBlock" { $scriptDefinition = { try { NonsenseString } catch { "An error occurred." } finally { "Finalizing..." } }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations | Should -BeNullOrEmpty } It "Catch as parameter" { $scriptDefinition = { Write-Host Catch }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations | Should -BeNullOrEmpty } It "Catch as double quoted string" { $scriptDefinition = { "Catch" }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations | Should -BeNullOrEmpty } It "Catch as single quoted string" { $scriptDefinition = { 'Catch' }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations | Should -BeNullOrEmpty } } @@ -115,7 +123,7 @@ Describe "MissingTryBlock" { catch { "An error occurred." } finally { "Finalizing..." } }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations | Should -BeNullOrEmpty } @@ -126,8 +134,25 @@ Describe "MissingTryBlock" { catch { "An error occurred." } finally { "Finalizing..." } }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations.Count | Should -Be 1 } } + + Context "Disabled" { + + BeforeAll { + $Settings = @{ + IncludeRules = @($ruleName) + Rules = @{ $ruleName = @{ Enable = $false } } + } + } + + It "ConvertFrom-SecureString -AsPlainText" { + $scriptDefinition = { catch { "An error occurred." } }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + } + } \ No newline at end of file diff --git a/docs/Rules/MissingTryBlock.md b/docs/Rules/MissingTryBlock.md index 2d4bc8831..52608a624 100644 --- a/docs/Rules/MissingTryBlock.md +++ b/docs/Rules/MissingTryBlock.md @@ -18,10 +18,19 @@ The `catch` and `finally` blocks must be preceded by a `try` block. Without a `t This rule identifies instances where `catch` or `finally` blocks are present with out an associated `try` block. +> [!NOTE] +> This rule is not enabled by default. The user needs to enable it through settings. + ## How Add a `try` block before the `catch` and `finally` blocks. +> [!NOTE] +> This rule could result in a false positive as it will fire on user code that violates the rule +> [AvoidReservedWordsAsFunctionNames][1] for functions named `catch` or `finally`: +> If you have functions named `catch` or `finally`, you can either rename the function or disable +> this rule. + ## Example ### Wrong @@ -36,3 +45,21 @@ catch { "An error occurred." } try { $a = 1 / $b } catch { "Attempted to divide by zero." } ``` + +## Configuration + +```powershell +Rules = @{ + PSAvoidExclaimOperator = @{ + Enable = $true + } +} +``` + +### Parameters + +- `Enable`: **bool** (Default value is `$false`) + + Enable or disable the rule during ScriptAnalyzer invocation. + +[1]: AvoidReservedWordsAsFunctionNames.md "Avoid using reserved words as function names." \ No newline at end of file diff --git a/docs/Rules/README.md b/docs/Rules/README.md index 41e7b8916..73e09a4da 100644 --- a/docs/Rules/README.md +++ b/docs/Rules/README.md @@ -50,7 +50,7 @@ The PSScriptAnalyzer contains the following rule definitions. | [DSCUseVerboseMessageInDSCResource](./DSCUseVerboseMessageInDSCResource.md) | Error | Yes | | | [MisleadingBacktick](./MisleadingBacktick.md) | Warning | Yes | | | [MissingModuleManifestField](./MissingModuleManifestField.md) | Warning | Yes | | -| [MissingTryBlock](./MissingTryBlock.md) | Warning | Yes | | +| [MissingTryBlock](./MissingTryBlock.md) | Warning | No | Yes | | [PlaceCloseBrace](./PlaceCloseBrace.md) | Warning | No | Yes | | [PlaceOpenBrace](./PlaceOpenBrace.md) | Warning | No | Yes | | [PossibleIncorrectComparisonWithNull](./PossibleIncorrectComparisonWithNull.md) | Warning | Yes | |