Add the AvoidSemicolonsAsLineTerminators rule to warn about lines ending with a semicolon. Fix (#824)#1806
Conversation
|
@michaeltlombardi Please review. And we need a docs issue opened for this. |
michaeltlombardi
left a comment
michaeltlombardi
left a comment
There was a problem hiding this comment.
Just a few small docs-related items. Thanks so much for this contribution, @tempora-mutantur!
| namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules | ||
| { | ||
| /// <summary> | ||
| /// AvoidSemicolonsAsLineTerminators: Checks for lines to don't end with semicolons |
There was a problem hiding this comment.
Minor suggestion for grammar and to bring synopsis into same structure as other rules.
| /// AvoidSemicolonsAsLineTerminators: Checks for lines to don't end with semicolons | |
| /// AvoidSemicolonsAsLineTerminators: Checks for lines that end with a semicolon. |
| } | ||
|
|
||
| /// <summary> | ||
| /// Analyzes the given ast to find violations. |
There was a problem hiding this comment.
Suggest clarifying specific behavior and standardizing to other rules.
| /// Analyzes the given ast to find violations. | |
| /// Checks for lines that end with a semicolon. |
| /// </summary> | ||
| /// <param name="ast">AST to be analyzed. This should be non-null</param> | ||
| /// <param name="fileName">Name of file that corresponds to the input AST.</param> | ||
| /// <returns>A an enumerable type containing the violations</returns> |
There was a problem hiding this comment.
Minor update to standardize to language used by other rules.
| /// <returns>A an enumerable type containing the violations</returns> | |
| /// <returns>The diagnostic results of this rule</returns> |
| } | ||
|
|
||
| /// <summary> | ||
| /// Retrieves the type of the rule, Builtin, Managed or Module. |
There was a problem hiding this comment.
Minor fix to standardize casing for valid keys with other items in this doc and to match docs for other rules.
| /// Retrieves the type of the rule, Builtin, Managed or Module. | |
| /// Retrieves the type of the rule: builtin, managed, or module. |
Rules/Strings.Designer.cs
Outdated
| } | ||
|
|
||
| /// <summary> | ||
| /// Looks up a localized string similar to Avoid long lines. |
There was a problem hiding this comment.
| /// Looks up a localized string similar to Avoid long lines. | |
| /// Looks up a localized string similar to Avoid semicolons as line terminators. | |
It's a rule to warn about lines ending with a semicolon
michaeltlombardi
left a comment
michaeltlombardi
left a comment
There was a problem hiding this comment.
LGTM! 💜
I filed a docs issue for tracking this on the other side once this PR is merged. 😊
|
Thank you all. Could you please advise what I should better do with the PSScriptAnalyzer-CI build failure, got a test failure for WS2022: Test Directed Graph.Runspaces should be disposed. Running analyzer 100 times should only create a limited number of runspaces Expected the actual value to be less than or equal to 14, because Number of Runspaces should be bound (size of runspace pool cache is 10), but got 15. I could be wrong, but it looks like flakiness. |
JamesWTruher
left a comment
JamesWTruher
left a comment
There was a problem hiding this comment.
this looks great - thanks for the contribution
5 participants
It's a rule to warn about lines ending with a semicolon
PR Summary
Add the PSAvoidSemicolonsAsLineTerminators rule to warn about lines ending with a semicolon. Covered with tests. The rule is disabled by default as it's more of a styling preference. #824
PR Checklist
.cs,.ps1and.psm1files have the correct copyright headerWIP:to the beginning of the title and remove the prefix when the PR is ready.