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

Refactor "extract method" false negative on break in loop inside the refactored code #1291

Open
Alberth289346 opened this issue Mar 29, 2024 · 1 comment · May be fixed by #1415
Open

Refactor "extract method" false negative on break in loop inside the refactored code #1291

Alberth289346 opened this issue Mar 29, 2024 · 1 comment · May be fixed by #1415
Assignees
Labels
bug Something isn't working
Milestone

Comments

@Alberth289346
Copy link

I was playing with eliminating left recursion in a grammar when JDT refused to extract a piece of code to a new method. I found this in Eclipse 2023-03, and confirmed it in 2024-03 as well.

There are two probably different refusals in the same original code, here I'll show the first one.

Reduced to a small example:

package completion2.grammar;

import java.util.List;

public class RefactorFalseNegativeBreakContinue {

    public static void falseNegativeBreakRefactor(List<List<String>> defs) {
        for (List<String> def: defs) {
            boolean isLeftRecursive = false;
            for (String rule: def) {
                if (!rule.isEmpty()) {
                    break;
                }
            }

            if (!isLeftRecursive) {
                continue;
            }
        }
    }
}

I want to refactor the body of the outer loop:

Message from Eclipse refactoring:
image

It looks asif JDT fails to see that the break is in an inner loop.

If either the break or the continue is removed, it works (although I haven't checked what it does then).

@Alberth289346 Alberth289346 changed the title Refactor false negative on break in loop inside the refactored code Refactor "extract method" false negative on break in loop inside the refactored code Mar 29, 2024
@Alberth289346
Copy link
Author

Changed title to also state "extract method" and added the second refactor bug in #1292

@jjohnstn jjohnstn self-assigned this May 21, 2024
@jjohnstn jjohnstn added the bug Something isn't working label May 21, 2024
@jjohnstn jjohnstn added this to the 4.33 M1 milestone May 21, 2024
jjohnstn pushed a commit to jjohnstn/eclipse.jdt.ui-1 that referenced this issue May 21, 2024
- fix ExtractMethodAnalyzer.canHandleBranches() to not flag a
  non-labelled break statement if the for loop is included
- add new test to ExtractMethodTests
- fixes eclipse-jdt#1291
@jjohnstn jjohnstn linked a pull request May 21, 2024 that will close this issue
3 tasks
jjohnstn added a commit to jjohnstn/eclipse.jdt.ui-1 that referenced this issue May 22, 2024
- fix ExtractMethodAnalyzer.canHandleBranches() to not flag a
  non-labelled break statement if the for loop is included
- add new test to ExtractMethodTests
- fixes eclipse-jdt#1291
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
Development

Successfully merging a pull request may close this issue.

2 participants