Skip to content

Fix length check in PSSnapinQualifiedName.GetInstance()#8837

Merged
iSazonov merged 2 commits intoPowerShell:masterfrom
hvitved:ps-snaping-qualified-name-get-instance
Feb 8, 2019
Merged

Fix length check in PSSnapinQualifiedName.GetInstance()#8837
iSazonov merged 2 commits intoPowerShell:masterfrom
hvitved:ps-snaping-qualified-name-get-instance

Conversation

@hvitved
Copy link
Copy Markdown
Contributor

@hvitved hvitved commented Feb 6, 2019

Hi,

I came across this when looking at the alerts for this project on LGTM.com (full disclosure: I work on the C# analysis there). The existing check splitName.Length < 0 is redundant, and I believe it should have been splitName.Length == 0, since the constructor of PSSnapinQualifiedName expects an array of length 1 or 2.

You can see the original alert on LGTM.com here: https://lgtm.com/projects/g/PowerShell/PowerShell/alerts/?mode=tree&ruleFocus=1506101336231

Best regards,
Tom

Copy link
Copy Markdown
Contributor

@bergmeister bergmeister left a comment

Choose a reason for hiding this comment

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

Snapins are not supported any more and this looks like left-over code. Maybe we should (if we can) add this file to the list files to be excluded from compilation in the csproj instead? There is a separate issue to cleanup snapin related code btw

@iSazonov
Copy link
Copy Markdown
Collaborator

iSazonov commented Feb 7, 2019

@bergmeister The code is used for modules. If I remember correctly most of SnapIn code already was removed.

@iSazonov
Copy link
Copy Markdown
Collaborator

iSazonov commented Feb 7, 2019

@hvitved Please add empty commit with '[Feature]' in the commit header to run all tests.

@iSazonov iSazonov self-assigned this Feb 7, 2019
@iSazonov iSazonov added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label Feb 7, 2019
@bergmeister
Copy link
Copy Markdown
Contributor

Ok, thanks. Sorry, this was not clear to me given the surrounding variable names.

@iSazonov iSazonov merged commit 5f59a9d into PowerShell:master Feb 8, 2019
@iSazonov
Copy link
Copy Markdown
Collaborator

iSazonov commented Feb 8, 2019

@hvitved Thanks for your contribution!

@hvitved hvitved deleted the ps-snaping-qualified-name-get-instance branch February 11, 2019 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants