Skip to content

Commit 97f30d6

Browse files
iRon7CopilotCopilotsdwheelerliamjpeters
authored
Add new MissingTryBlock rule (#2179)
* #2098 Add new MissingTryBlock rule * 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 <copilot@github.com> * Updated tests for MissingTryBlock rule to reflect severity change from Error to Warning. * Update Rules/MissingTryBlock.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Grammar: “which is likely a mistake and result in … error” is ungrammatical. * Update Rules/Strings.resx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update Rules/MissingTryBlock.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * minor but worthwhile * Update docs/Rules/MissingTryBlock.md Co-authored-by: Sean Wheeler <sean.wheeler@microsoft.com> * 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. * Update docs/Rules/MissingTryBlock.md Co-authored-by: Liam Peters <liamjpeters@gmail.com> * Update Tests/Rules/MissingTryBlock.tests.ps1 Co-authored-by: Liam Peters <liamjpeters@gmail.com> * Update Rules/MissingTryBlock.cs Co-authored-by: Liam Peters <liamjpeters@gmail.com> * Update Rules/MissingTryBlock.cs Co-authored-by: Liam Peters <liamjpeters@gmail.com> --------- Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Sean Wheeler <sean.wheeler@microsoft.com> Co-authored-by: Liam Peters <liamjpeters@gmail.com>
1 parent 9c04a44 commit 97f30d6

5 files changed

Lines changed: 364 additions & 0 deletions

File tree

‎Rules/MissingTryBlock.cs‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
#if !CORECLR
7+
using System.ComponentModel.Composition;
8+
#endif
9+
using System.Globalization;
10+
using System.Management.Automation.Language;
11+
using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
12+
13+
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
14+
{
15+
#if !CORECLR
16+
[Export(typeof(IScriptRule))]
17+
#endif
18+
19+
/// <summary>
20+
/// Rule that warns when catch or finally blocks are used without a corresponding try block
21+
/// </summary>
22+
23+
public class MissingTryBlock : ConfigurableRule
24+
{
25+
26+
/// <summary>
27+
/// Construct an object of MissingTryBlock type.
28+
/// </summary>
29+
public MissingTryBlock() {
30+
Enable = false;
31+
}
32+
33+
/// <summary>
34+
/// Find bare word "catch" or "finally" tokens that are not part of a TryStatementAst
35+
/// </summary>
36+
/// <param name="ast">AST to be analyzed. This should be non-null</param>
37+
/// <param name="fileName">Name of file that corresponds to the input AST.</param>
38+
/// <returns>A an enumerable type containing the violations</returns>
39+
public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
40+
{
41+
if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage);
42+
43+
// Find the bare word 'catch' or 'finally' StringConstantExpressionAst nodes used as commands
44+
var missingTryAsts = ast.FindAll(testAst =>
45+
// Normally should be part of a TryStatementAst
46+
testAst is StringConstantExpressionAst stringAst &&
47+
// Check whether "catch" or "finally" are bare words
48+
stringAst.StringConstantType == StringConstantType.BareWord &&
49+
(
50+
String.Equals(stringAst.Value, "catch", StringComparison.OrdinalIgnoreCase) ||
51+
String.Equals(stringAst.Value, "finally", StringComparison.OrdinalIgnoreCase)
52+
) &&
53+
stringAst.Parent is CommandAst commandAst &&
54+
// Only violate if the catch or finally is the first command element
55+
commandAst.CommandElements[0] == stringAst,
56+
true
57+
);
58+
59+
foreach (StringConstantExpressionAst missingTryAst in missingTryAsts)
60+
{
61+
yield return new DiagnosticRecord(
62+
string.Format(
63+
CultureInfo.CurrentCulture,
64+
Strings.MissingTryBlockError,
65+
CultureInfo.CurrentCulture.TextInfo.ToTitleCase(missingTryAst.Value)),
66+
missingTryAst.Extent,
67+
GetName(),
68+
DiagnosticSeverity.Warning,
69+
fileName,
70+
missingTryAst.Value
71+
);
72+
}
73+
}
74+
75+
/// <summary>
76+
/// Retrieves the common name of this rule.
77+
/// </summary>
78+
public override string GetCommonName()
79+
{
80+
return string.Format(CultureInfo.CurrentCulture, Strings.MissingTryBlockCommonName);
81+
}
82+
83+
/// <summary>
84+
/// Retrieves the description of this rule.
85+
/// </summary>
86+
public override string GetDescription()
87+
{
88+
return string.Format(CultureInfo.CurrentCulture, Strings.MissingTryBlockDescription);
89+
}
90+
91+
/// <summary>
92+
/// Retrieves the name of this rule.
93+
/// </summary>
94+
public override string GetName()
95+
{
96+
return string.Format(
97+
CultureInfo.CurrentCulture,
98+
Strings.NameSpaceFormat,
99+
GetSourceName(),
100+
Strings.MissingTryBlockName);
101+
}
102+
103+
/// <summary>
104+
/// Retrieves the severity of the rule: error, warning or information.
105+
/// </summary>
106+
public override RuleSeverity GetSeverity()
107+
{
108+
return RuleSeverity.Warning;
109+
}
110+
111+
/// <summary>
112+
/// Retrieves the name of the module/assembly the rule is from.
113+
/// </summary>
114+
public override string GetSourceName()
115+
{
116+
return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
117+
}
118+
119+
/// <summary>
120+
/// Retrieves the type of the rule, Builtin, Managed or Module.
121+
/// </summary>
122+
public override SourceType GetSourceType()
123+
{
124+
return SourceType.Builtin;
125+
}
126+
}
127+
}
128+

‎Rules/Strings.resx‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,18 @@
276276
<data name="MissingModuleManifestFieldCommonName" xml:space="preserve">
277277
<value>Module Manifest Fields</value>
278278
</data>
279+
<data name="MissingTryBlockName" xml:space="preserve">
280+
<value>MissingTryBlock</value>
281+
</data>
282+
<data name="MissingTryBlockCommonName" xml:space="preserve">
283+
<value>Missing Try Block</value>
284+
</data>
285+
<data name="MissingTryBlockDescription" xml:space="preserve">
286+
<value>The catch and finally blocks should be preceded by a try block.</value>
287+
</data>
288+
<data name="MissingTryBlockError" xml:space="preserve">
289+
<value>{0} is missing a try block</value>
290+
</data>
279291
<data name="AvoidUnloadableModuleDescription" xml:space="preserve">
280292
<value>If a script file is in a PowerShell module folder, then that folder must be loadable.</value>
281293
</data>

‎Tests/Rules/MissingTryBlock.tests.ps1‎

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
# Copyright (c) Microsoft Corporation. All rights reserved.
2+
# Licensed under the MIT License.
3+
4+
[Diagnostics.CodeAnalysis.SuppressMessage('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'False positive')]
5+
param()
6+
7+
BeforeAll {
8+
$ruleName = "PSMissingTryBlock"
9+
}
10+
11+
Describe "MissingTryBlock" {
12+
13+
BeforeAll {
14+
$Settings = @{
15+
IncludeRules = @($ruleName)
16+
Rules = @{ $ruleName = @{ Enable = $true } }
17+
}
18+
}
19+
20+
Context "Violates" {
21+
It "Catch is missing a try block" {
22+
$scriptDefinition = { catch { "An error occurred." } }.ToString()
23+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
24+
$violations.Count | Should -Be 1
25+
$violations.Severity | Should -Be Warning
26+
$violations.Extent.Text | Should -Be catch
27+
$violations.Message | Should -Be 'Catch is missing a try block'
28+
$violations.RuleSuppressionID | Should -Be catch
29+
}
30+
31+
It "Finally is missing a try block" {
32+
$scriptDefinition = { finally { "Finalizing..." } }.ToString()
33+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
34+
$violations.Count | Should -Be 1
35+
$violations.Severity | Should -Be Warning
36+
$violations.Extent.Text | Should -Be finally
37+
$violations.Message | Should -Be 'Finally is missing a try block'
38+
$violations.RuleSuppressionID | Should -Be finally
39+
}
40+
41+
It "Single line catch and finally is missing a try block" {
42+
$scriptDefinition = {
43+
catch { "An error occurred." } finally { "Finalizing..." }
44+
}.ToString()
45+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
46+
$violations.Count | Should -Be 1
47+
$violations.Severity | Should -Be Warning
48+
$violations.Extent.Text | Should -Be catch
49+
$violations.Message | Should -Be 'Catch is missing a try block'
50+
$violations.RuleSuppressionID | Should -Be catch
51+
}
52+
53+
It "Multi line catch and finally is missing a try block" {
54+
$scriptDefinition = {
55+
catch { "An error occurred." }
56+
finally { "Finalizing..." }
57+
}.ToString()
58+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
59+
$violations.Count | Should -Be 2
60+
$violations[0].Severity | Should -Be Warning
61+
$violations[0].Extent.Text | Should -Be catch
62+
$violations[0].Message | Should -Be 'Catch is missing a try block'
63+
$violations[0].RuleSuppressionID | Should -Be catch
64+
$violations[1].Severity | Should -Be Warning
65+
$violations[1].Extent.Text | Should -Be finally
66+
$violations[1].Message | Should -Be 'Finally is missing a try block'
67+
$violations[1].RuleSuppressionID | Should -Be finally
68+
}
69+
}
70+
71+
Context "Compliant" {
72+
It "try-catch block" {
73+
$scriptDefinition = {
74+
try { NonsenseString }
75+
catch { "An error occurred." }
76+
}.ToString()
77+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
78+
$violations | Should -BeNullOrEmpty
79+
}
80+
81+
It "try-catch-final statement" {
82+
$scriptDefinition = {
83+
try { NonsenseString }
84+
catch { "An error occurred." }
85+
finally { "Finalizing..." }
86+
}.ToString()
87+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
88+
$violations | Should -BeNullOrEmpty
89+
}
90+
91+
It "Single line try statement" {
92+
$scriptDefinition = {
93+
try { NonsenseString } catch { "An error occurred." } finally { "Finalizing..." }
94+
}.ToString()
95+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
96+
$violations | Should -BeNullOrEmpty
97+
}
98+
99+
It "Catch as parameter" {
100+
$scriptDefinition = { Write-Host Catch }.ToString()
101+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
102+
$violations | Should -BeNullOrEmpty
103+
}
104+
105+
It "Catch as double quoted string" {
106+
$scriptDefinition = { "Catch" }.ToString()
107+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
108+
$violations | Should -BeNullOrEmpty
109+
}
110+
111+
It "Catch as single quoted string" {
112+
$scriptDefinition = { 'Catch' }.ToString()
113+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
114+
$violations | Should -BeNullOrEmpty
115+
}
116+
}
117+
118+
Context "Suppressed" {
119+
It "Multi line catch and finally is missing a try block" {
120+
$scriptDefinition = {
121+
[Diagnostics.CodeAnalysis.SuppressMessage('PSMissingTryBlock', '', Justification = 'Test')]
122+
param()
123+
catch { "An error occurred." }
124+
finally { "Finalizing..." }
125+
}.ToString()
126+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
127+
$violations | Should -BeNullOrEmpty
128+
}
129+
130+
It "Multi line catch and finally is missing a try block for catch only" {
131+
$scriptDefinition = {
132+
[Diagnostics.CodeAnalysis.SuppressMessage('PSMissingTryBlock', 'finally', Justification = 'Test')]
133+
param()
134+
catch { "An error occurred." }
135+
finally { "Finalizing..." }
136+
}.ToString()
137+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
138+
$violations.Count | Should -Be 1
139+
}
140+
}
141+
142+
Context "Disabled" {
143+
144+
BeforeAll {
145+
$Settings = @{
146+
IncludeRules = @($ruleName)
147+
Rules = @{ $ruleName = @{ Enable = $false } }
148+
}
149+
}
150+
151+
It "Doesn't emit a violation" {
152+
$scriptDefinition = { catch { "An error occurred." } }.ToString()
153+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
154+
$violations | Should -BeNullOrEmpty
155+
}
156+
}
157+
158+
}

‎docs/Rules/MissingTryBlock.md‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
---
2+
description: Missing Try Block
3+
ms.date: 04/22/2026
4+
ms.topic: reference
5+
title: MissingTryBlock
6+
---
7+
# MissingTryBlock
8+
9+
**Severity Level: Warning**
10+
11+
## Description
12+
13+
The `catch` and `finally` blocks must be preceded by a `try` block. Without a `try` block, the
14+
`catch` and `finally` are interpreted as commands and result in a runtime error, such as:
15+
16+
> "The term 'catch' is not recognized as a name of a cmdlet"
17+
18+
This rule identifies instances where `catch` or `finally` blocks are present with out an associated
19+
`try` block.
20+
21+
> [!NOTE]
22+
> This rule is not enabled by default. The user needs to enable it through settings.
23+
24+
## How
25+
26+
Add a `try` block before the `catch` and `finally` blocks.
27+
28+
> [!NOTE]
29+
> This rule could result in a false positive as it will fire on user code that violates the rule
30+
> [AvoidReservedWordsAsFunctionNames][1] for functions named `catch` or `finally`:
31+
> If you have functions named `catch` or `finally`, you can either rename the function or disable
32+
> this rule.
33+
34+
## Example
35+
36+
### Wrong
37+
38+
```powershell
39+
catch { "An error occurred." }
40+
```
41+
42+
### Correct
43+
44+
```powershell
45+
try { $a = 1 / $b }
46+
catch { "Attempted to divide by zero." }
47+
```
48+
49+
## Configuration
50+
51+
```powershell
52+
Rules = @{
53+
PSMissingTryBlock = @{
54+
Enable = $true
55+
}
56+
}
57+
```
58+
59+
### Parameters
60+
61+
- `Enable`: **bool** (Default value is `$false`)
62+
63+
Enable or disable the rule during ScriptAnalyzer invocation.
64+
65+
[1]: AvoidReservedWordsAsFunctionNames.md "Avoid using reserved words as function names."

‎docs/Rules/README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ The PSScriptAnalyzer contains the following rule definitions.
5050
| [DSCUseVerboseMessageInDSCResource](./DSCUseVerboseMessageInDSCResource.md) | Error | Yes | |
5151
| [MisleadingBacktick](./MisleadingBacktick.md) | Warning | Yes | |
5252
| [MissingModuleManifestField](./MissingModuleManifestField.md) | Warning | Yes | |
53+
| [MissingTryBlock](./MissingTryBlock.md) | Warning | No | Yes |
5354
| [PlaceCloseBrace](./PlaceCloseBrace.md) | Warning | No | Yes |
5455
| [PlaceOpenBrace](./PlaceOpenBrace.md) | Warning | No | Yes |
5556
| [PossibleIncorrectComparisonWithNull](./PossibleIncorrectComparisonWithNull.md) | Warning | Yes | |

0 commit comments

Comments
 (0)