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 local variables in 'for' #1292

Open
Alberth289346 opened this issue Mar 29, 2024 · 0 comments
Open
Assignees
Labels
bug Something isn't working
Milestone

Comments

@Alberth289346
Copy link

This is the 2nd refactor bug in the same original code as #1291 after working around the first refusal.

Encountered in Eclipse 2023-03, and confirmed in 2024-03. It seems related to my previous bug report #671 except that was in while context.

I assumed the cause for both bugs are different, so I simplified the original code independent from what I did in #1291. That lead to

package completion2.grammar;

import java.util.ArrayList;
import java.util.List;

public class RefactorFalseNegativeLocalVars {

    public static void falseNegativeLocalVarsRefactor(List<String> defs) {
        List<String> newDefs = new ArrayList<>();

        for (String rule: defs) {
            boolean isLeftRecursive = false;
            if (!isLeftRecursive) {
                continue;
            }

            List<String> oldRules = new ArrayList<String>();
            List<String> newSuffixes = new ArrayList<String>();

            newDefs.addAll(oldRules);
            newDefs.addAll(newSuffixes);
        }
    }
}

When you try to extract the body of the loop to a new method, you get
image

This is a wrong conclusion since the local variables are local to the extracted method, and cease to exist at the end of that body. If the code performs continue, I would hope the local oldRules and newSuffixes variables are also created from scratch again, but even if they don't, they are initialized in the declaration after the continue, so any old data in them would be destroyed.

@jjohnstn jjohnstn self-assigned this May 22, 2024
@jjohnstn jjohnstn added the bug Something isn't working label May 22, 2024
@jjohnstn jjohnstn added this to the 4.33 M1 milestone May 22, 2024
jjohnstn added a commit to jjohnstn/eclipse.jdt.ui-1 that referenced this issue May 22, 2024
- fix ExtractMethodAnalyzer.computeOutput() method to add a check
  for local variables that have been added to the returnValues list
  but the declaration and all references are within the selected
  region
- add new test to ExtractMethodTests
- fixes eclipse-jdt#1292
jjohnstn added a commit to jjohnstn/eclipse.jdt.ui-1 that referenced this issue May 22, 2024
- fix ExtractMethodAnalyzer.computeOutput() method to add a check
  for local variables that have been added to the returnValues list
  but the declaration and all references are within the selected
  region
- add new test to ExtractMethodTests
- fixes eclipse-jdt#1292
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

No branches or pull requests

2 participants