From 6e41b236725eedc9bf35e1be2232ef21e58cc10f Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Thu, 8 May 2025 17:54:09 +0200 Subject: [PATCH] [extract method with object] show error when variables are used in loop (IDEA-356602) GitOrigin-RevId: e84bcf1f1d03819ca4b5c12b91fad72baf82fc27 --- .../parameterObject/ParameterObjectUtils.kt | 18 +++++++++++++++--- .../IntroduceObjectFailedWithLoop.java | 12 ++++++++++++ .../ExtractMethodAndDuplicatesInplaceTest.kt | 8 +++++++- 3 files changed, 34 insertions(+), 4 deletions(-) create mode 100644 java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/IntroduceObjectFailedWithLoop.java diff --git a/java/java-impl-refactorings/src/com/intellij/refactoring/extractMethod/newImpl/parameterObject/ParameterObjectUtils.kt b/java/java-impl-refactorings/src/com/intellij/refactoring/extractMethod/newImpl/parameterObject/ParameterObjectUtils.kt index 07eb51b9699c..6bead6d9489e 100644 --- a/java/java-impl-refactorings/src/com/intellij/refactoring/extractMethod/newImpl/parameterObject/ParameterObjectUtils.kt +++ b/java/java-impl-refactorings/src/com/intellij/refactoring/extractMethod/newImpl/parameterObject/ParameterObjectUtils.kt @@ -29,17 +29,29 @@ object ParameterObjectUtils { } private fun findAffectedReferences(variable: PsiVariable, scope: List): List? { - val startingOffset = scope.last().textRange.endOffset + val parent = scope.first().parent + val beforeScope = scope.first().textRange.startOffset + val afterScope = scope.last().textRange.endOffset val references = ReferencesSearch.search(variable) .asIterable() .mapNotNull { it.element as? PsiReferenceExpression } - .filter { reference -> reference.textRange.startOffset >= startingOffset } + .filter { reference -> reference.textRange.startOffset >= afterScope || reference.textRange.endOffset <= beforeScope } .sortedBy { reference -> reference.textRange.startOffset } + val referencesBefore = references.filter { ref -> ref.textRange.endOffset <= beforeScope } + if (!referencesBefore.isEmpty()) { + val loop = PsiTreeUtil.getParentOfType(parent, PsiLoopStatement::class.java, true) + if (loop != null) { + val suspiciousLoop = referencesBefore.any { ref -> PsiTreeUtil.isAncestor(loop, ref, true) } + if (suspiciousLoop) return null + } + } val firstAssignment = references.find { reference -> PsiUtil.isAccessedForWriting(reference) } ?: return references val assignmentExpression = PsiTreeUtil.getParentOfType(firstAssignment, PsiAssignmentExpression::class.java) if (assignmentExpression == null) return null if (assignmentExpression.parent.parent != PsiTreeUtil.findCommonParent(assignmentExpression, scope.last())) return null - return references.filter { reference -> reference.textRange.endOffset <= assignmentExpression.textRange.endOffset } - firstAssignment + return references.filter { ref -> ref != firstAssignment + && ref.textRange.endOffset <= assignmentExpression.textRange.endOffset + && ref.textRange.startOffset >= afterScope } } } \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/IntroduceObjectFailedWithLoop.java b/java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/IntroduceObjectFailedWithLoop.java new file mode 100644 index 000000000000..0c6f9118dfab --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/IntroduceObjectFailedWithLoop.java @@ -0,0 +1,12 @@ +public class Test { + + void test() { + double x = 0; + double y = 0; + while (x < 10) { + x = x + .1; + y = y - .1; + } + System.out.println("x: " + x + "; y: " + y); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodAndDuplicatesInplaceTest.kt b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodAndDuplicatesInplaceTest.kt index 7ac74935be94..9653ab2aa2f3 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodAndDuplicatesInplaceTest.kt +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodAndDuplicatesInplaceTest.kt @@ -1,4 +1,4 @@ -// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.intellij.java.refactoring import com.intellij.codeInsight.hint.HintManager @@ -443,6 +443,12 @@ class ExtractMethodAndDuplicatesInplaceTest: LightJavaCodeInsightTestCase() { require(getActiveTemplate() != null) } + fun testIntroduceObjectFailedWithLoop(){ + assertThrows(RefactoringErrorHintException::class.java) { + doTest() + } + } + fun testIntroduceObjectFailedWithAssignment1(){ assertThrows(RefactoringErrorHintException::class.java) { doTest()