Skip to content

Extract Method: top level method that is static should create a static extraction#58021

Merged
akhera99 merged 8 commits intodotnet:mainfrom
akhera99:bugs/58013_extract_method
Mar 15, 2022
Merged

Extract Method: top level method that is static should create a static extraction#58021
akhera99 merged 8 commits intodotnet:mainfrom
akhera99:bugs/58013_extract_method

Conversation

@akhera99
Copy link
Copy Markdown
Member

Fixes: #58013

@ghost ghost added the Area-IDE label Nov 30, 2021
{
if (method.IsStatic)
{
tokens.Add(SyntaxFactory.Token(SyntaxKind.StaticKeyword));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this feels odd. shouldn't this always be added if hte method is static? to the point that i think we could just have this check before everything else, and unilaterally add 'static' (regardless oc ompilation-unit/type/interface destinatino). thoughts?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

hrmm... i see. there's an issue of wanting to avoid things like accessibility.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i think this should move up to line 281 and become an 'else' of htat condition.

}
}

if (destination is CodeGenerationDestination.CompilationUnit)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not replace the following condition (around line 239):

                if (destination is not CodeGenerationDestination.CompilationUnit and
                    not CodeGenerationDestination.Namespace and
                    not CodeGenerationDestination.InterfaceType)

with

                if (destination is not CodeGenerationDestination.Namespace and
                    not CodeGenerationDestination.InterfaceType)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

so compilation unit locations can't support things like accessibility.

@akhera99 akhera99 marked this pull request as ready for review November 30, 2021 22:45
@akhera99 akhera99 requested a review from a team as a code owner November 30, 2021 22:45
@akhera99 akhera99 enabled auto-merge March 15, 2022 16:59
@akhera99 akhera99 merged commit 0b8d848 into dotnet:main Mar 15, 2022
@ghost ghost added this to the Next milestone Mar 15, 2022
@allisonchou allisonchou modified the milestones: Next, 17.2.P3 Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extract method of part of static local method creates non static method causing compile error

5 participants