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

Use Pattern Matching suggestion gives faulty code #72370

Closed
Joy-less opened this issue Mar 2, 2024 · 5 comments · Fixed by #72466
Closed

Use Pattern Matching suggestion gives faulty code #72370

Joy-less opened this issue Mar 2, 2024 · 5 comments · Fixed by #72466
Labels
Area-IDE Bug help wanted The issue is "up for grabs" - add a comment if you are interested in working on it
Milestone

Comments

@Joy-less
Copy link

Joy-less commented Mar 2, 2024

Version Used: C# 12

Steps to Reproduce:

  1. Paste the following code into Visual Studio:
public class Program {
    public class Cat {
    }
    public static void Main() {
        Cat Cat = new();
        object Object = Cat;
        
        // Suggestion: IDE0083: Use Pattern Matching
        if (!(Object is Cat)) {
            
        }
    }
}
  1. If you accept the suggestion, it corrects your code to:
if (Object is not Cat) {
    
}

which gives you a compilation error: (line 14, col 21): A constant value is expected

Diagnostic Id: IDE0083

Expected Behavior:
No suggestion given.

Actual Behavior:
Faulty suggestion given.

Related discussion:
dotnet/csharplang#7980

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Mar 2, 2024
@CyrusNajmabadi CyrusNajmabadi added Bug 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 Mar 2, 2024
@CyrusNajmabadi CyrusNajmabadi added this to the Backlog milestone Mar 2, 2024
@ghost
Copy link

ghost commented Mar 5, 2024

Hey, I could take this, but I need some information.

I think in this case, you would try to remove the ambiguity by putting a more qualified name such as Program.Cat here, but I don't know if there is logic dedicated to handling this type of syntax generation. If there is, I would like to know more.

Also, I would like to know how one would manage the logic of only putting enough qualification to the name to resolve the ambiguity, but not necessarily the fully qualified one as it might not be fully necessary.

@CyrusNajmabadi
Copy link
Member

I think in this case, you would try to remove the ambiguity by putting a more qualified name such as Program.Cat here

That seems reasonable. Or we could block this case in the analyzer.

but I don't know if there is logic dedicated to handling this type of syntax generation

You can use the extension GenerateNameSyntax for this purpose.

Also, I would like to know how one would manage the logic of only putting enough qualification to the name to resolve the ambiguity, but not necessarily the fully qualified one as it might not be fully necessary.

GenerateNameSyntax will add an annotation to the node asking our simplifier to simplify it.

@ghost
Copy link

ghost commented Mar 7, 2024

I think in this case, you would try to remove the ambiguity by putting a more qualified name such as Program.Cat here

That seems reasonable. Or we could block this case in the analyzer.

but I don't know if there is logic dedicated to handling this type of syntax generation

You can use the extension GenerateNameSyntax for this purpose.

Also, I would like to know how one would manage the logic of only putting enough qualification to the name to resolve the ambiguity, but not necessarily the fully qualified one as it might not be fully necessary.

GenerateNameSyntax will add an annotation to the node asking our simplifier to simplify it.

I experimented using generator.NameExpression which ends up doing that and it does give me the fully qualified name which works, but now I am hitting a second issue: the simplifier reduces it back to the same ambiguous name.

So it seems that there's a part of this where the simplification is uneeded, but now comes the question of how this can be done? It might be possible to manually walk up the names until there's no collisions manually, but now the question becomes how do I know a given name collides?

@CyrusNajmabadi
Copy link
Member

the simplifier reduces it back to the same ambiguous name.

The simplifier should be fixed as part of the PR. Thanks :)

@CyrusNajmabadi
Copy link
Member

but now the question becomes how do I know a given name collides?

The simplifier can check for this with the name choices it makes. Specifically looking to see if the simple name binds to something like this in a context like this and jsut blacklisting it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Bug 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.

2 participants