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

CS4033 creates invalid code on partial methods #63404

Closed
MisinformedDNA opened this issue Aug 15, 2022 · 2 comments · Fixed by #76168
Closed

CS4033 creates invalid code on partial methods #63404

MisinformedDNA opened this issue Aug 15, 2022 · 2 comments · Fixed by #76168
Labels
Area-IDE help wanted The issue is "up for grabs" - add a comment if you are interested in working on it
Milestone

Comments

@MisinformedDNA
Copy link

Version Used: VS 2022 17.3.0

Steps to Reproduce:

  1. Implement a partial method
  2. Add an async call
  3. Use code fix "Make method async (stay void)"
using System.Threading.Tasks;

public partial class C {
    partial void M();
}

public partial class C {
    partial void M() {
        await Task.Delay(1);
    }
}

Expected Behavior:

Generates valid code:

    async partial void M() {
        await Task.Delay(1);
    }

Actual Behavior:

Generates invalid code. async needs to come before partial

    partial async void M() {
        await Task.Delay(1);
    }
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Aug 15, 2022
@Youssef1313
Copy link
Member

@CyrusNajmabadi Should we respect editorconfig's modifier order preference as part of the cleanup process that's run on every code action?

internal static async Task<Document> CleanupDocumentAsync(
Document document, CodeCleanupOptions options, CancellationToken cancellationToken)
{
document = await ImportAdder.AddImportsFromSymbolAnnotationAsync(
document, Simplifier.AddImportsAnnotation, options.AddImportOptions, cancellationToken).ConfigureAwait(false);
document = await Simplifier.ReduceAsync(document, Simplifier.Annotation, options.SimplifierOptions, cancellationToken).ConfigureAwait(false);
// format any node with explicit formatter annotation
document = await Formatter.FormatAsync(document, Formatter.Annotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false);
// format any elastic whitespace
document = await Formatter.FormatAsync(document, SyntaxAnnotation.ElasticAnnotation, options.FormattingOptions, cancellationToken).ConfigureAwait(false);
document = await CaseCorrector.CaseCorrectAsync(document, CaseCorrector.Annotation, cancellationToken).ConfigureAwait(false);
return document;
}

possibly via a reducer?

private static readonly ImmutableArray<AbstractReducer> s_reducers =
ImmutableArray.Create<AbstractReducer>(
new CSharpVarReducer(),
new CSharpNameReducer(),
new CSharpNullableAnnotationReducer(),
new CSharpCastReducer(),
new CSharpExtensionMethodReducer(),
new CSharpParenthesizedExpressionReducer(),
new CSharpParenthesizedPatternReducer(),
new CSharpEscapingReducer(),
new CSharpMiscellaneousReducer(),
new CSharpInferredMemberNameReducer(),
new CSharpDefaultExpressionReducer());

@dibarbet
Copy link
Member

This seems to be broken in general as it also does not make the other partial definition return Task. We should also respect order modifiers, but we should also generate non-broken code (which I think we can here).

image

@dibarbet dibarbet added help wanted The issue is "up for grabs" - add a comment if you are interested in working on it and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Aug 19, 2022
@dibarbet dibarbet added this to the Backlog milestone Aug 19, 2022
@github-project-automation github-project-automation bot moved this to InQueue in Small Fixes Oct 22, 2024
@github-project-automation github-project-automation bot moved this from InQueue to Completed in Small Fixes Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE help wanted The issue is "up for grabs" - add a comment if you are interested in working on it
Projects
Status: Completed
Development

Successfully merging a pull request may close this issue.

4 participants