Address Set-Location/Get-Item Issue With ?/* Characters

[NoMoreFood]

PR Summary

• Adjusted behavior of GetCorrectCasedPath() which attempts to search for the file on the file system in order to normalize its case. Since some file systems support wildcard-type characters (e.g. MacOS) in path names so these need to be skipped to avoid erroneous resolution.
• Added tests to detect this behavior.

PR Context

This corrects the behavior where Set-Location fails to change directory or changes to an entirely different child directory when the target directory contains characters such as '*'. Similarly, Get-Item also returns errant information.

Resolves: #12299

PR Checklist

• PR has a meaningful title
  • Use the present tense and imperative mood when describing your changes
• Summarized changes
• Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
• This PR is ready to merge and is not Work in Progress.
  • If the PR is work in progress, please add the prefix WIP: or [ WIP ] to the beginning of the title (the WIP bot will keep its status check at Pending while the prefix is present) and remove the prefix when the PR is ready.
• Breaking changes
  • None
  • OR
  • Experimental feature(s) needed
    • Experimental feature name(s):
• User-facing changes
  • Not Applicable
  • OR
  • Documentation needed
    • Issue filed:
• Testing - New and feature
  • N/A or can only be tested interactively
  • OR
  • Make sure you've added a new test if existing tests do not effectively test the code changed
• Tooling
  • I have considered the user experience from a tooling perspective and don't believe tooling will be impacted.
  • OR
  • I have considered the user experience from a tooling perspective and enumerated concerns in the summary. This may include:
    • Impact on PowerShell Editor Services which is used in the PowerShell extension for VSCode (which runs in a different PS Host).
    • Impact on Completions (both in the console and in editors) - one of PowerShell's most powerful features.
    • Impact on PSScriptAnalyzer (which provides linting & formatting in the editor extensions).
    • Impact on EditorSyntax (which provides syntax highlighting with in VSCode, GitHub, and many other editors).

[TravisEz13]

This look good. Thanks.

I've requested a coup of changes in the tests to make sure it doesn't regress things now or in the future.

[TravisEz13]

@PowerShell/powershell-committee This is a breaking change. Please review. We made a similar change for matching commands. Where we prefer literal matches over wildcard matches.

[NoMoreFood]

@TravisEz13 So I can categorize these correctly in the future, can you educate me as to why this would be considered a breaking change?

[SteveL-MSFT]

@PowerShell/powershell-committee agrees that the current behavior is broken where:

set-location -literalpath ?

must treat ? as a literal value and not a wildcard. We don't believe this is a breaking change that causes concern, however, we are concerned with the location of the code change in a helper that is used by many code paths which itself may introduce a regression somewhere else.

[TravisEz13]

I'm not sure your change addressed the committee's concern about the location of the change.

[NoMoreFood]

@TravisEz13 Sorry, the day job has distracted me from following through on this and one other PR. I honestly thought the "location of the code change" comment was sort of meant to be rhetorical. The bug itself can be scoped to that helper function: In what scenario would I possibly pass a literal, existing directory path to GetCorrectCasedPath() and it would be acceptable to get back a reference to an existing different directory? The function exists solely to normalize case within a path --- it should NEVER be returning a path to a different file.

[ghost]

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Mainainer, Please provide feedback and/or mark it as Waiting on Author

[ghost]

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Mainainer, Please provide feedback and/or mark it as Waiting on Author

[NoMoreFood]

Is this ready for merge or are we still waiting for reviews? @TravisEz13

[NoMoreFood]

I believe I have resolved these.

[TravisEz13]

@TravisEz13 Sorry, the day job has distracted me from following through on this and one other PR. I honestly thought the "location of the code change" comment was sort of meant to be rhetorical. The bug itself can be scoped to that helper function: In what scenario would I possibly pass a literal, existing directory path to GetCorrectCasedPath() and it would be acceptable to get back a reference to an existing different directory? The function exists solely to normalize case within a path --- it should NEVER be returning a path to a different file.

Here is the direct call:
https://sourcegraph.com/search?q=repo:powershell/powershell+GetCorrectCasedPath&patternType=literal

Then, NormalizePath is used in these places:
https://sourcegraph.com/search?q=repo:powershell/powershell+content:%22Microsoft.PowerShell.Commands%22+content:%22NormalizePath%28%22+lang:c%23++-file:%22src/System.Management.Automation/namespaces/RegistryProvider.cs%22&patternType=literal#1

We need to make sure we didn't break any of these calls.

[TravisEz13]

I'll let the Provider working group review from here.

[iSazonov]

The GetCorrectCasedPath() is not in right place. This would only have to be used in Formatting System and completors.

[NoMoreFood]

@TravisEz13 All of the non-file system paths you reference end up calling a base class version or provider-specific version of NormalizePath() so this change luckily does not affect them.

@iSazonov I think that might be splitting hairs to some degree. When somebody does a Set-Location to a specific path, do you want the resultant path at the prompt to be case-accurate or not? I think it might be hard to address all the "what if" sort of cases. Regardless, this PR was authored to address the "bug" within GetCorrectCasedPath() itself so I was hoping not to boil the ocean to get this committed. :-)

[ghost]

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

[ghost]

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author