diff --git a/Rules/MissingTryBlock.cs b/Rules/MissingTryBlock.cs new file mode 100644 index 000000000..dd48149ae --- /dev/null +++ b/Rules/MissingTryBlock.cs @@ -0,0 +1,140 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +#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 + + /// + /// Rule that warns when catch or finally blocks are used without a corresponding try block + /// + + public class MissingTryBlock : ConfigurableRule + { + + /// + /// Construct an object of MissingTryBlock type. + /// + 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); + + // 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 && + // Check whether "catch" or "finally" are 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.Warning, + fileName, + missingTryAst.Value + ); + } + } + + /// + /// Retrieves the common name of this rule. + /// + public override string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.MissingTryBlockCommonName); + } + + /// + /// Retrieves the description of this rule. + /// + public override string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.MissingTryBlockDescription); + } + + /// + /// Retrieves the name of this rule. + /// + public override string GetName() + { + return string.Format( + CultureInfo.CurrentCulture, + Strings.NameSpaceFormat, + GetSourceName(), + Strings.MissingTryBlockName); + } + + /// + /// 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; + } + + /// + /// Retrieves the name of the module/assembly the rule is from. + /// + public override string GetSourceName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + } + + /// + /// Retrieves the type of the rule, Builtin, Managed or Module. + /// + public override SourceType GetSourceType() + { + return SourceType.Builtin; + } + } +} + diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 2a04fd759..a296b04f3 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..7a06e4d04 --- /dev/null +++ b/Tests/Rules/MissingTryBlock.tests.ps1 @@ -0,0 +1,158 @@ +# 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" { + + 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 -Settings $Settings + $violations.Count | Should -Be 1 + $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 + } + + It "Finally is missing a try block" { + $scriptDefinition = { finally { "Finalizing..." } }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.Count | Should -Be 1 + $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 + } + + It "Single line catch and finally is missing a try block" { + $scriptDefinition = { + catch { "An error occurred." } finally { "Finalizing..." } + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.Count | Should -Be 1 + $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 + } + + It "Multi line catch and finally is missing a try block" { + $scriptDefinition = { + catch { "An error occurred." } + finally { "Finalizing..." } + }.ToString() + $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 + $violations[0].Message | Should -Be 'Catch is missing a try block' + $violations[0].RuleSuppressionID | Should -Be catch + $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 + } + } + + Context "Compliant" { + It "try-catch block" { + $scriptDefinition = { + try { NonsenseString } + catch { "An error occurred." } + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + + It "try-catch-final statement" { + $scriptDefinition = { + try { NonsenseString } + catch { "An error occurred." } + finally { "Finalizing..." } + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + + It "Single line try statement" { + $scriptDefinition = { + try { NonsenseString } catch { "An error occurred." } finally { "Finalizing..." } + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + + It "Catch as parameter" { + $scriptDefinition = { Write-Host Catch }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + + It "Catch as double quoted string" { + $scriptDefinition = { "Catch" }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + + It "Catch as single quoted string" { + $scriptDefinition = { 'Catch' }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $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 -Settings $Settings + $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 -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 new file mode 100644 index 000000000..52608a624 --- /dev/null +++ b/docs/Rules/MissingTryBlock.md @@ -0,0 +1,65 @@ +--- +description: Missing Try Block +ms.date: 04/22/2026 +ms.topic: reference +title: MissingTryBlock +--- +# MissingTryBlock + +**Severity Level: Warning** + +## Description + +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. + +> [!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 + +```powershell +catch { "An error occurred." } +``` + +### Correct + +```powershell +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 fca031e33..73e09a4da 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) | Warning | No | Yes | | [PlaceCloseBrace](./PlaceCloseBrace.md) | Warning | No | Yes | | [PlaceOpenBrace](./PlaceOpenBrace.md) | Warning | No | Yes | | [PossibleIncorrectComparisonWithNull](./PossibleIncorrectComparisonWithNull.md) | Warning | Yes | |