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

No Type completions on a switch case when the resolved type of switch expression is non-primitive in Java17+ #1301

Open
gayanper opened this issue Aug 21, 2023 · 6 comments

Comments

@gayanper
Copy link
Contributor

In the following code

public class Mapper {
  public Integer mapJobState(String value) {
    return switch (value) {
      case 
    };
  }
}

Code completions behind case M doesn't provide any completions.

@gayanper
Copy link
Contributor Author

This is a Java17 and above issue

gayanper added a commit to gayanper/eclipse.jdt.core that referenced this issue Aug 25, 2023
gayanper added a commit to gayanper/eclipse.jdt.core that referenced this issue Sep 14, 2023
gayanper added a commit to gayanper/eclipse.jdt.core that referenced this issue Sep 17, 2023
gayanper added a commit to gayanper/eclipse.jdt.core that referenced this issue Sep 22, 2023
gayanper added a commit to gayanper/eclipse.jdt.core that referenced this issue Sep 23, 2023
gayanper added a commit to gayanper/eclipse.jdt.core that referenced this issue Sep 30, 2023
gayanper added a commit to gayanper/eclipse.jdt.core that referenced this issue Oct 6, 2023
@gayanper
Copy link
Contributor Author

@stephan-herrmann The PR was made under the following assumptions

When we write a switch block with a int variable

switch(i) {
   case Const
}

completions behind Const will provide proposals where user can choose the class where they want to select a constant. if they don't do that, the compiler will issue a error.

Now I would expect the same for a switch with a string variable like

switch(str) {
   case Const
}

But due to the code segment at https://github.com/eclipse-jdt/eclipse.jdt.core/blob/master/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/CompletionEngine.java#L3967
which cause the scenario with str variable not to work. because the else condition at https://github.com/eclipse-jdt/eclipse.jdt.core/blob/master/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/CompletionEngine.java#L3980 cause only to have switch block related keyword completions.

So the question is should we handle this scenario ? I feel we should since it is working for primitive and why not for string ? And the next question is why only limiting few keywords as completions when there is a non primitive resolved type in Java17 ? shouldn't the pattern specific errors must be handled by compiler ? And if we really want to support choosing right types in java17+ pattern matching, should we implement it based on completion relevance instead ?

@gayanper
Copy link
Contributor Author

Now I trace down the issue where this new code changes were introduced which is from 1f29892#diff-8469e0f05a7f350e7047101a3e20fe0bf4d0f65df01fa0a18eb6378128c5de5a

Could it be that we missed to add a default completion in the new if block ? @vik-chand

@gayanper gayanper changed the title No code completion on string switch case No Type completions on a switch case when the resolved type of switch expression is non-primitive in Java17+ Oct 16, 2023
@gayanper
Copy link
Contributor Author

@vik-chand @stephan-herrmann any thoughts on above mentioned findings.

I'm thinking of restoring pre Java 17 behaviour together with support for Java17 specific keywords to start with.

@stephan-herrmann
Copy link
Contributor

@vik-chand @stephan-herrmann any thoughts on above mentioned findings.

I'm thinking of restoring pre Java 17 behaviour together with support for Java17 specific keywords to start with.

Meanwhile I filed #1531 for the slightly bigger picture. Should we fail to resolve that issue in time for M3, I'm fine with merging the point-fix in #1312.

@gayanper
Copy link
Contributor Author

gayanper commented Oct 29, 2023

@stephan-herrmann agree, I think this is not a big deal, I mean the the issue I report here is one use case which a developer will not come across many times. So I'm leaning towards closing my PR, because it seems it does only handle one use case where we use string, this could given a wrong impression on fixing others scenarios later for other contributors.

So lets try to find what is expected when a switch is not more a primitive expression and see how to provide a better solution in the issue you reported. I will put some ideas and arguments I have in mind regarding this.

Let me know what you think, then we can close the PR attached to this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants