Use nameof operator part2 #13086

xtqqczze · 2020-07-02T15:53:55Z

PR Summary

"paramName" -> nameof(paramName)
Fix paramName typos/casing/name issues

Previously this PR had more changes, but they were removed for a future follow-up PR.

PR Context

Follow-up: #12716

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
- [NA] Experimental feature(s) needed
  - [NA] Experimental feature name(s):
User-facing changes
- Not Applicable
- OR
- [NA] Documentation needed
  - [NA] Issue filed:
Testing - New and feature
- N/A or can only be tested interactively
- OR
- [NA] 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
- [NA] 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).

iSazonov · 2020-07-03T09:22:27Z

@xtqqczze Microsoft.Management.UI.Internal is a frozen code. Please exclude it from the PR.

xtqqczze · 2020-07-05T15:34:18Z

@iSazonov Code cleanup changes have previously been merged to Microsoft.Management.UI.Internal (#12917), why must these changes be excluded?

iSazonov · 2020-07-06T11:14:08Z

This code should not be changed. It will not develop, it is not tested.

xtqqczze · 2020-07-06T17:49:43Z

@iSazonov Rebased to remove changes to Microsoft.Management.UI.Internal.

src/System.Management.Automation/engine/remoting/fanin/OutOfProcTransportManager.cs

src/System.Management.Automation/security/CredentialParameter.cs

src/Microsoft.PowerShell.Commands.Utility/commands/utility/MatchString.cs

src/System.Management.Automation/engine/runtime/MutableTuple.cs

src/System.Management.Automation/utils/MshTraceSource.cs

iSazonov · 2020-07-07T05:04:27Z

src/System.Management.Automation/FormatAndOutput/common/BaseOutputtingCommand.cs


            if (fed.formatEntryInfo == null)
            {
-                PSTraceSource.NewArgumentNullException("fed.formatEntryInfo");


I wonder why do we change this? Please revert.

We should not use ArgumentNullException here as the argument is not null but a property.

Intention is to trace. We shouldn't remove this.

I wonder why we do not throw the exception from NewArgumentNullException here. I think this could be a bug.

iSazonov · 2020-07-07T05:04:38Z

...ystem.Management.Automation/FormatAndOutput/common/DisplayDatabase/displayDescriptionData.cs


            if (string.IsNullOrEmpty(Value))
                if (Value == null || ValueType == DisplayEntryValueType.Property)
-                    throw PSTraceSource.NewArgumentNullException(nameof(expression));


I wonder why do we change this? Please revert.

We should not use ArgumentNullException here as the argument expression may not be null.

Intention is to trace. We shouldn't remove this.

Documentation for ArgumentNullException states:

An ArgumentNullException exception is thrown when a method is invoked and at least one of the passed arguments is null but should never be null.

The passed argument expression may not be null when we throw here so ArgumentNullException is not appropriate.

iSazonov · 2020-07-07T05:05:26Z

src/System.Management.Automation/engine/MshCommandRuntime.cs

            ActionPreference preference = pair.Value;
            if (errorRecord == null)
            {
-                throw PSTraceSource.NewArgumentNullException("errorRecord");


Why not NewArgumentNullException(nameof(errorRecord))?

Method signature is DoWriteError(object obj), errorRecord is not an argument.

Intention is to trace. We shouldn't remove this.

We should not pass "errorRecord" as paramName as it is not an argument for this method.

The question is not for the style PR.

ghost · 2020-07-26T20:00:24Z

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 15 days. It will be closed if no further activity occurs within 10 days of this comment.

iSazonov · 2020-07-27T07:17:25Z

@xtqqczze Please address last comments.

xtqqczze · 2020-08-03T20:08:07Z

@iSazonov Addressed

ghost · 2020-08-19T02:00:30Z

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

daxian-dbw · 2020-09-01T22:08:58Z

@PowerShell/powershell-maintainers reviewed this PR and agreed to accept it.

ghost · 2020-09-09T14:00:14Z

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

TravisEz13

Let's start with a PR where all the names end up the change and no types are changed. There is too much too review here.

TravisEz13 · 2020-09-09T17:55:42Z

src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHostRawUserInterface.cs

+        /// <param name="str"></param>
        /// <param name="offset"></param>
        /// <returns></returns>
        /// <exception cref="HostException">
        /// If Win32's WideCharToMultiByte fails
        /// </exception>

        public override
-        int LengthInBufferCells(string s, int offset)
+        int LengthInBufferCells(string str, int offset)
        {
-            if (s == null)
+            if (str == null)
            {
-                throw PSTraceSource.NewArgumentNullException("str");
+                throw PSTraceSource.NewArgumentNullException(nameof(str));
            }

-            return ConsoleControl.LengthInBufferCells(s, offset, parent.SupportsVirtualTerminal);
+            return ConsoleControl.LengthInBufferCells(str, offset, parent.SupportsVirtualTerminal);


Please try to keep different changes in different PRs

TravisEz13 · 2020-09-09T17:57:28Z

src/System.Management.Automation/FormatAndOutput/common/BaseOutputtingCommand.cs

            if (fed.formatEntryInfo == null)
            {
-                PSTraceSource.NewArgumentNullException("fed.formatEntryInfo");
+                PSTraceSource.NewArgumentException(nameof(fed), "fed.formatEntryInfo is null");


Please move any changes which change an exception type to a new PR

TravisEz13 · 2020-09-09T17:58:05Z

src/System.Management.Automation/DscSupport/CimDSCParser.cs

            {
 #if CORECLR
-                throw PSTraceSource.NewArgumentException("isConfiguration", ParserStrings.ConfigurationNotSupportedInPowerShellCore);
+                throw PSTraceSource.NewArgumentException(nameof(moduleInfo), ParserStrings.ConfigurationNotSupportedInPowerShellCore);


This doesn't look right

TravisEz13 · 2020-09-09T17:59:06Z

src/System.Management.Automation/FormatAndOutput/common/DisplayDatabase/typeDataXmlLoader.cs

        {
            if (typeDefinition == null)
-                throw PSTraceSource.NewArgumentNullException("viewDefinition");
+                throw PSTraceSource.NewArgumentNullException(nameof(typeDefinition));


again, why is this changed

TravisEz13 · 2020-09-09T17:59:33Z

src/System.Management.Automation/engine/CommandDiscovery.cs

            if (newCmdletInfo == null)
            {
-                throw PSTraceSource.NewArgumentNullException("cmdlet");
+                throw PSTraceSource.NewArgumentNullException(nameof(newCmdletInfo));


TravisEz13 · 2020-09-09T17:59:43Z

src/System.Management.Automation/engine/FunctionInfo.cs

            if (newFunction == null)
            {
-                throw PSTraceSource.NewArgumentNullException("function");
+                throw PSTraceSource.NewArgumentNullException(nameof(newFunction));


TravisEz13 · 2020-09-09T18:00:22Z

src/System.Management.Automation/engine/MshSnapinQualifiedName.cs

                // a bad format.

-                throw PSTraceSource.NewArgumentException("name");
+                throw PSTraceSource.NewArgumentException(nameof(splitName));


ghost · 2020-09-24T20:00:05Z

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 15 days. It will be closed if no further activity occurs within 10 days of this comment.

xtqqczze · 2020-10-12T23:42:42Z

@iSazonov I have made changes to this PR, could you please reopen.

iSazonov · 2020-10-13T04:34:59Z

I can not reopen :-( - GitHub button is blocked. Do you remove the PR branch?

iSazonov · 2020-10-16T04:21:07Z

@xtqqczze Please create new PR.

xtqqczze · 2020-10-25T21:45:24Z

Opened #13875

xtqqczze · 2020-10-27T02:12:10Z

See also #13898 for ArgumentException changes.

ghost assigned TravisEz13 Jul 2, 2020

xtqqczze force-pushed the use-nameof-operator2 branch 3 times, most recently from 5505fac to 8695fba Compare July 2, 2020 19:57

xtqqczze added 3 commits July 6, 2020 18:48

"value" -> nameof(value)

7b08f70

"paramName" -> nameof(paramName)

e4b26de

Fix paramName parameter

361ddb6

xtqqczze force-pushed the use-nameof-operator2 branch from 8695fba to 361ddb6 Compare July 6, 2020 17:49

iSazonov reviewed Jul 6, 2020

View reviewed changes

xtqqczze added 3 commits July 6, 2020 20:24

Address @iSazonov feedback

7f3a9cf

Fix incorrect ArgumentNullException

b7f69e4

fixup! Fix paramName parameter

7c6efc9

xtqqczze force-pushed the use-nameof-operator2 branch from d8d1a73 to 7c6efc9 Compare July 6, 2020 21:07

iSazonov reviewed Jul 7, 2020

View reviewed changes

src/System.Management.Automation/utils/MshTraceSource.cs Outdated Show resolved Hide resolved

iSazonov reviewed Jul 7, 2020

View reviewed changes

ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jul 11, 2020

ghost added the Stale label Jul 26, 2020

ghost removed the Stale label Jul 27, 2020

TravisEz13 added the Stale label Jul 31, 2020

ghost removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept Stale labels Aug 3, 2020

xtqqczze requested review from SteveL-MSFT, TravisEz13, adityapatwardhan, anmenaga and daxian-dbw as code owners August 3, 2020 20:11

restore ArgumentException

739d413

TravisEz13 assigned daxian-dbw and unassigned TravisEz13 Aug 11, 2020

ghost added the Review - Needed The PR is being reviewed label Aug 19, 2020

daxian-dbw added the Review - Maintainer The PR/issue needs a review from the PowerShell repo Maintainers label Aug 19, 2020

ghost removed the Review - Needed The PR is being reviewed label Sep 1, 2020

daxian-dbw removed the Review - Maintainer The PR/issue needs a review from the PowerShell repo Maintainers label Sep 1, 2020

iSazonov approved these changes Sep 2, 2020

View reviewed changes

ghost added the Review - Needed The PR is being reviewed label Sep 9, 2020

TravisEz13 requested changes Sep 9, 2020

View reviewed changes

ghost added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Review - Needed The PR is being reviewed labels Sep 9, 2020

ghost added the Stale label Sep 24, 2020

ghost closed this Oct 5, 2020

ghost removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept Stale labels Oct 12, 2020

This pull request was closed.

Use nameof operator part2 #13086

Use nameof operator part2 #13086

Uh oh!

Conversation

xtqqczze commented Jul 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Context

PR Checklist

Uh oh!

iSazonov commented Jul 3, 2020

Uh oh!

xtqqczze commented Jul 5, 2020

Uh oh!

iSazonov commented Jul 6, 2020

Uh oh!

xtqqczze commented Jul 6, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xtqqczze Aug 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Jul 26, 2020

Uh oh!

iSazonov commented Jul 27, 2020

Uh oh!

xtqqczze commented Aug 3, 2020

Uh oh!

ghost commented Aug 19, 2020

Uh oh!

daxian-dbw commented Sep 1, 2020

Uh oh!

ghost commented Sep 9, 2020

Uh oh!

TravisEz13 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

xtqqczze commented Jul 2, 2020 •

edited

Loading

xtqqczze Aug 3, 2020 •

edited

Loading

xtqqczze commented Oct 27, 2020 •

edited

Loading