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

Single if cleanup should not concatenate instanceofs that use pattern matching #1200

Closed
jjohnstn opened this issue Feb 15, 2024 · 2 comments · Fixed by #1201 or #1931
Closed

Single if cleanup should not concatenate instanceofs that use pattern matching #1200

jjohnstn opened this issue Feb 15, 2024 · 2 comments · Fixed by #1201 or #1931
Assignees
Labels
bug Something isn't working
Milestone

Comments

@jjohnstn
Copy link
Contributor

jjohnstn commented Feb 15, 2024

Running Single if cleanup on the following:

public class BadInstanceOfMerge {
	protected String getTipWidget(Number number) {

		if (number instanceof Long n) {
			return n.toString();
		}
		if (number instanceof Float n) {
			return n.toString();
		}
		if (number instanceof Double n) {
			return n.toString();
		}

		return "";
	}
}

results in:

public class BadInstanceOfMerge {
	protected String getTipWidget(Number number) {

		if ((number instanceof Long n) || (number instanceof Float n) || (number instanceof Double n)) {
			return n.toString();
		}

		return "";
	}
}

part of #1197

@jjohnstn jjohnstn self-assigned this Feb 15, 2024
@jjohnstn jjohnstn added the bug Something isn't working label Feb 15, 2024
@jjohnstn jjohnstn added this to the 4.32 M1 milestone Feb 15, 2024
jjohnstn added a commit to jjohnstn/eclipse.jdt.ui-1 that referenced this issue Feb 16, 2024
- fixes eclipse-jdt#1200
- add new PatternInstanceof logic to
  OneIfRatherThanDuplicateBlocksThatFallThroughFixCore
- add new test to CleanUpTest16
jjohnstn added a commit to jjohnstn/eclipse.jdt.ui-1 that referenced this issue Mar 9, 2024
- fixes eclipse-jdt#1200
- add new PatternInstanceof logic to
  OneIfRatherThanDuplicateBlocksThatFallThroughFixCore
- add new test to CleanUpTest16
jjohnstn added a commit that referenced this issue Mar 9, 2024
* Fix one if cleanup to watch for PatternInstanceof nodes

- fixes #1200
- add new PatternInstanceof logic to
  OneIfRatherThanDuplicateBlocksThatFallThroughFixCore
- add new test to CleanUpTest16
@dg444
Copy link

dg444 commented Jan 11, 2025

This issue is not fixed in Eclipse 4.34.0 2024-12.
Example1:
if (number instanceof Long n) {
return n.toString();
}
else if (number instanceof Float n) {
return n.toString();
}
else if (number instanceof Double n) {
return n.toString();
}
=> fails to compile:
if ((number instanceof Long n) || (number instanceof Float n) || (number instanceof Double n)) {
return n.toString();
}
Example 2:
if (e instanceof IOException io)
throw io;
else if (e.getCause() instanceof IOException io)
throw io;
=> fails to compile:
f ((e instanceof IOException io) || (e.getCause() instanceof IOException io))
throw io;

@jjohnstn jjohnstn reopened this Jan 13, 2025
@jjohnstn jjohnstn modified the milestones: 4.32 M1, 4.35 M2 Jan 13, 2025
@jjohnstn
Copy link
Contributor Author

There are actually 2 cleanups referenced here. One cleanup combines the multiple ifs as mentioned in the original description. That cleanup was fixed. The other cleanup merges conditions of an if/else if/else. That issue (Bugzilla 570171) should not have been closed. I will post a fix shortly for 2025-03 M2

jjohnstn added a commit to jjohnstn/eclipse.jdt.ui-1 that referenced this issue Jan 13, 2025
- fix MergeConditionalBlocksCleanup to handle instanceof patterns and
  not merge them when they have same name
- add new test to CleanUpTest16
- fixes eclipse-jdt#1200
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants