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.

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

Extract local function: Variable shadowing prior CSharp 8 #55135

Closed

Conversation

MaStr11
Copy link
Contributor

@MaStr11 MaStr11 commented Jul 27, 2021

Fix #55031

C# 8 allows shadowing of variables by local function parameters. The Extract local function refactoring takes this for granted. The following code is causing CS0136 (A local variable named 'var' cannot be declared in this scope because it would give a different meaning to 'var', which is already used in a 'parent or current/child' scope to denote something else) in C# 7.3 and older:

Before

        static void Main(string[] args)
        {
            var f = "{0}";
            string.Format(f, ""); // Extract local function
        }

After

        static void Main(string[] args)
        {
            var f = "{0}";
            NewMethod(f);

            void NewMethod(string f) // Valid in C#8 but causes CS0136 in C#7.3.
            {
                string.Format(f, "");
            }
        }

@MaStr11 MaStr11 requested a review from a team as a code owner July 27, 2021 08:59
@MaStr11
Copy link
Contributor Author

MaStr11 commented Jul 27, 2021

@CyrusNajmabadi What is the expected behavior:

Name the function parameter f1

        static void Main(string[] args)
        {
            var f = "{0}";
            NewMethod(f);

            void NewMethod(string f1)
            {
                string.Format(f1, "");
            }
        }

Capture f

        static void Main(string[] args)
        {
            var f = "{0}";
            NewMethod();

            void NewMethod()
            {
                string.Format(f, "");
            }
        }

var f = ""{0}"";
{|Rename:NewMethod|}(f);

void NewMethod(string f)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

f is causing CS0136 in C# 7.3. The refactoring should either name the parameter differently or capture f.

@CyrusNajmabadi
Copy link
Member

Capturing or renaming both seem reasonable. I'd like the refactoring to offer both options.

@jinujoseph jinujoseph added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Jul 27, 2021
@ryzngard
Copy link
Contributor

#55446 has the fix to rename the variable here by default. If we want to capture as a different option we should file an issue for adding that

@MaStr11
Copy link
Contributor Author

MaStr11 commented Aug 20, 2021

#55446 uses the capturing approach. I'm going to close this PR in favor of #55446 because it is almost done and it fixes the issue. If anyone sees value in adding the "pass parameters (with renaming if before C# 8 or only offer passing if C# 8 and above)", that should be added after #55446 is merged. But I agree, that this is a separate issue.

@MaStr11
Copy link
Contributor Author

MaStr11 commented Aug 24, 2021

Closed in favor of #55446

@MaStr11 MaStr11 closed this Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor to create local method creates argument for local const member
4 participants