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

Fix parsing the -Settings object as a path when the path object originates from an expression #915

Merged
merged 3 commits into from
Mar 5, 2018
Merged

Fix parsing the -Settings object as a path when the path object originates from an expression #915

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5cd3dfe
Fix parsing the -Settings object as a path when the path is an expres…
bergmeister Mar 1, 2018
da48f8a
address PR comments, improve verbose logging and tidy up
bergmeister Mar 3, 2018
f2eada8
Improve check for file type by trying to cast to PSObject and checkin…
bergmeister Mar 4, 2018
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
Next Next commit
Fix parsing the -Settings object as a path when the path is an expres… 
…sion that is not fully evaluated yet.
  • Loading branch information
@bergmeister
bergmeister committed Mar 1, 2018
commit 5cd3dfecd35660679e92553f933b6e48191ce460
4 changes: 4 additions & 0 deletions Engine/Settings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,10 @@ internal static SettingsMode FindSettingsMode(object settings, string path, out
{
settingsMode = SettingsMode.Hashtable;
}
else // if the provided object is wrapped in multiple expressions then it might not be resolved yet at this stage -> try using the File type as a last resort and best guess.
{
settingsMode = SettingsMode.File;
}
}
}

Expand Down
12 changes: 12 additions & 0 deletions Tests/Engine/InvokeScriptAnalyzer.tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,18 @@ Describe "Test CustomizedRulePath" {
}
}

It "Should process relative settings path even when settings path object is an expression" {
try {
$initialLocation = Get-Location
Set-Location $PSScriptRoot
Copy link
Contributor

Choose a reason for hiding this comment

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

this should probably be Push-Location, that way you don't need to save the location at all.

$warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'gci' -Settings (Join-Path (Get-Location).Path '.\SettingsTest\..\SettingsTest\Project1\PSScriptAnalyzerSettings.psd1')
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this - in this case, the execution of Join-Path happens before the cmdlet is ever called, so this is just result of the join-path rather than an actual expression.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am not sure if it is really Join-Path because (Join-path a b).GetType() returns a string. As far as I understood when stepping through it in the debugger is that because we are in BeginProcessing(), the passed in object is not resolved to a string yet but the code was assuming that in the case of files that the input can be casted to a string in order to determine that it is of type 'file'.

Copy link
Contributor

Choose a reason for hiding this comment

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

the place to look is in the setter of the parameter value. You could set a breakpoint in the setter (around line 215 of InvokeScriptAnalyzerCommand.cs) and inspect the value (or Settings after it's been set). The way powershell should be doing this is because of the parens, that statement is evaluated and then the parameter is bound to the string output of join-path. I haven't walked this through the debugger, but the way to check is that if Settings is the string result of the join-path (rather than the whole statement with parens) the check above should be definitive.

Copy link
Collaborator Author

@bergmeister bergmeister Mar 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

You are right that PowerShell evaluates it in the setter but the object is of type System.Management.Automation.PSObject, whereas our property is just System.Object and therefore .Net truthfully says that it is not of type string. What I could do is try to cast it to PSObject then and check whether its BaseObject property is of type string,

Copy link
Contributor

Choose a reason for hiding this comment

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

if it winds up being not a string, that's good enough.

$warnings.Count | Should -Be 1
}
finally {
Set-Location $initialLocation
Copy link
Contributor

Choose a reason for hiding this comment

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

Pop-Location

}
}

It "Should use the CustomRulePath parameter" {
$settings = @{
CustomRulePath = "$directory\CommunityAnalyzerRules"
Expand Down