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

[content assist] Suboptimal/No code completion suggestions for switching on sealed interfaces #3300

Closed
jubax opened this issue Nov 12, 2024 · 10 comments · Fixed by #3302
Closed
Assignees

Comments

@jubax
Copy link

jubax commented Nov 12, 2024

If you have a class with a "sealed interface" then neither the autocomplete nor the quickfix handles that well.

public class SealedInterfaceContentAssist {

	sealed interface Foo {
		record FooImpl_a(String x) implements Foo {
		}

		record FooImpl_b(String y, String z) implements Foo {
		}
	}

	public static void main(String[] args) {
		Foo foo = getFoo();
		switch (foo) { // <== No quickfix for creating the case labels
		}
	}

	private static Foo getFoo() {
		return new Foo.FooImpl_b("a", "b");
	}

}

Sadly there is no auto-generation of the switch labels. So I am trying it myself:

	switch (foo) {
	case FooIm
	}

So I type <ctrl+space> after typing "FooIm", but there is no suggestion. Even after I type case FooImpl_a() I do not get any completion.

In the end I had to type manually all that code by myself:

	public static void main(String[] args) {
		Foo foo = getFoo();
		switch (foo) {
			case FooImpl_a(String x) -> System.out.println();
			case FooImpl_b(String y, String z) -> System.out.println();
		}
	}

It also seems that Eclipse shows multiple compile errors around the whole switch statement (e.g. missing default case) until I fully implemented all case labels. So users who are not 100% familiar with the syntax for such a switch statement are lost.

@srikanth-sankaran
Copy link
Contributor

srikanth-sankaran commented Nov 12, 2024

There are three issues entwined here.

  1. Could you add to this ticket [Switch Expressions] Poor quality diagnostics  #3289 examples showing poor quality diagnostics ?

  2. For any quick fix/assist please raise a ticket against JDT UI. It is handled there.

  3. Let us use this ticket for code completion. I agree with the adjective "sad". The days of having dedicated experts for code assist is long gone and the same small number of people are having to don multiple hats :-(

I'll take a look to see what low hanging opportunities can be harnessed.

Perhaps it is too late for 4.34. Let us see.

Thanks for the defect report.

Also please make sure you are on latest version. On the compiler side there are lots of defect fixes coming in 4.34.

@jubax
Copy link
Author

jubax commented Nov 12, 2024

There are three issues entwined here.

1. Could you add to this ticket [[Switch Expressions] Poor quality diagnostics  #3289](https://github.com/eclipse-jdt/eclipse.jdt.core/issues/3289) examples showing poor quality diagnostics ?

I'm not 100% sure what you mean. I think the link between the two issues has already been established.

2. For any quick fix/assist please raise a ticket against JDT UI. It is handled there.

I created eclipse-jdt/eclipse.jdt.ui#1795.

3. Let us use this ticket for code completion. I agree with the adjective "sad". The days of having dedicated experts for code assist is long gone and the same small number of people are having to don multiple hats :-(

If you don’t mind me asking, what do you think are the reasons for that? Change of focus for the Eclipse Foundation or in general lack of developers interested in that area of the code?

I'll take a look to see what low hanging opportunities can be harnessed.

Perhaps it is too late for 4.34. Let us see.

Thanks for the defect report.

Also please make sure you are on latest version. On the compiler side there are lots of defect fixes coming in 4.34.

Thank you very much for your work! Content assist is something where even minor improvements are noticed. I am currently using 4.33 and will upgrade as soon as 4.34 is available. I will update the ticket for 4.34.

@nlisker
Copy link

nlisker commented Nov 12, 2024

Isn't this the same as #1531?

@srikanth-sankaran
Copy link
Contributor

Isn't this the same as #1531?

Yes, some progress has been merged there, I will see what a comprehensive solution looks like for 4.35 M1

@srikanth-sankaran
Copy link
Contributor

Proper test case (the one post above is invalid as the record classes are not in scope at the completion point and it would be incorrect to suggest them on account of it)

public sealed interface Foo {
	record FooImpl_a(String x) implements Foo {
	}

	record FooImpl_b(String y, String z) implements Foo {
	}
		
	public static void main(String[] args) {
		Foo foo = getFoo();
		switch (foo) {
			case Foo  // offers no completion here.
		}
	}

	private static Foo getFoo() {
		return new Foo.FooImpl_b("a", "b");
	}
}

@srikanth-sankaran
Copy link
Contributor

It also seems that Eclipse shows multiple compile errors around the whole switch statement (e.g. missing default case) until I fully implemented all case labels. So users who are not 100% familiar with the syntax for such a switch statement are lost.

The compiler or the editor (compilation unit resolver to be precise) reports errors on code as it sees it as it is evolving keystroke by keystroke.

In particular the editor gives constant feedback by continually resolving the editor contents (resolving = scanning + parsing + building AST + performing semantic analysis and type checking). It does this for every buffer change - a single keystroke that dirties the buffer triggers the loop above and results in diagnostics shown.

This cannot be profitably suppressed without losing functionality. How will the editor know when you have completed typing so it can give you diagnostics ??

But if on the other hand while the code is still being typed if there are too many distracting messages, it can be due to poor quality diagnostics - instead of just pointing out the key flaw, the compiler could be reporting too many secondary or tertiary errors. If that is observed to happen, I am saying you can add snippets showing such compiler behavior to here

@srikanth-sankaran
Copy link
Contributor

(You may want to disable the preference "Report problems as you type" in Window + Preferences + Java + Editor but that is significant loss of functionality)

@srikanth-sankaran srikanth-sankaran changed the title [content assist] Suboptimal content assist and quickfix suggestion for sealed interfaces [content assist] Suboptimal/No code completion suggestions for switching on sealed interfaces Nov 13, 2024
srikanth-sankaran added a commit to srikanth-sankaran/eclipse.jdt.core that referenced this issue Nov 13, 2024
@srikanth-sankaran
Copy link
Contributor

OK, good news. The fix to make proposals appear for the types is straightforward. i.e., if you attempt completion at

case Foo<here> you will get choices for FooImpl_a, FooImpl_b etc - but still no record component autofills. I'll integrate this incremental improvement for 4.34. Everything else will have to wait for comprehensive stock taking and action in the context of #1531 for 4.35 M1 - it is too late for 4.34.

srikanth-sankaran added a commit to srikanth-sankaran/eclipse.jdt.core that referenced this issue Nov 13, 2024
@srikanth-sankaran
Copy link
Contributor

The proposed fix moves it from NO proposals to suboptimal territory. By that I mean there is no filtering applied based on the selector type. Completion engine doesn't yet recognize it is a sealed interface and only propose the permitted types in cases etc.
All that will have to wait for #1531

@srikanth-sankaran
Copy link
Contributor

So I will use this ticket just for the point fix proposed in current PR which would allow type name completions in a switch label and close this when #3302 is integrated

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