Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ErrorView to SpecialVars.cs #1865

Merged
merged 2 commits into from
Sep 17, 2023
Merged

Add ErrorView to SpecialVars.cs #1865

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cfe0055
add ErrorView to SpecialVars and create test
ewisniew0 Nov 22, 2022
b0f0486
Changed type of ErrorView
ewisniew0 Dec 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Engine/SpecialVars.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ static SpecialVars()
internal const string ConfirmPreference = "ConfirmPreference";
internal const string ProgressPreference = "ProgressPreference";
internal const string InformationPreference = "InformationPreference";
internal const string ErrorView = "ErrorView";

internal static readonly string[] PreferenceVariables = new string[]
{
Expand All @@ -101,7 +102,8 @@ static SpecialVars()
WarningPreference,
ConfirmPreference,
ProgressPreference,
InformationPreference
InformationPreference,
ErrorView
};

internal static readonly Type[] PreferenceVariableTypes = new Type[]
Expand All @@ -114,6 +116,7 @@ static SpecialVars()
/* ConfirmPreference */ typeof(ConfirmImpact),
/* ProgressPreference */ typeof(Enum),
/* InformationPreference */ typeof(ActionPreference),
/* ErrorView */ typeof(Enum), //ErrorView type not available on PS3
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is PSV3 variable defined as we do conditional compilation for each version of PS (3, 4, 5 and 'Core'). You could use this to use Enum only for the v3 compiled version but the correct types for others.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I can do that, however I'm still not sure why this type array is here since nobody seems to reference it, should it be expanded despite this?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bergmeister @JamesWTruher what is the point of this enum? Can we just delete the whole thing, and merge this PR already?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jaykul Whilst it's not directly referenced, it is still needed as it clearly solves the issue of not flagging false positive for ErrorView. I presume the rule is using reflection. The PSV3 variable is a pragma, i.e. a build time variable as we use different PowerShell version SDKs when we build against the different versions.

Copy link

@Jaykul Jaykul May 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, maybe we should probably just type this as string.

The ErrorView enum is new in PS7 and it's not enforced. The variable can be overwritten, and now that it's an Enum in PS7, it has to be overwritten by anyone who wants to improve the error view (such as my ErrorView module, which actually works on old versions of PowerShell)...

};

internal enum AutomaticVariable
Expand Down
5 changes: 5 additions & 0 deletions Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ function MyFunc2() {
}

Context "When there are no violations" {
It "No warning is issued for assignment without use of preference variable ErrorView" {
$results = Invoke-ScriptAnalyzer -ScriptDefinition '$ErrorView = NormalView'
$results.Count | Should -Be 0
}

It "returns no violations" {
$noViolations.Count | Should -Be 0
}
Expand Down