Description Lorenzo Puglioli 2023-06-08 08:12:17 EDT

Steps To Reproduce:
1. Create a class with this code:

public class TestExtractMethod {

	public void testMethod() {
		TestClass testClass = null;
		try {
			//Start here
			testClass = testClassFactoryPotentiallyFailing();
			methodPotentiallyThrowingException();
			//End here
		} finally {
			if (testClass != null) {
				testClass.doSomething();
			}
		}

	}

	private TestClass testClassFactoryPotentiallyFailing() {
		return new TestClass();
	}

	private void methodPotentiallyThrowingException() {
		//something...
	}

	private static class TestClass {

		public void doSomething() {
			//something
		}

	}

}

2. Select the lines between "Start here" and "End here" comments
3. Apply the refactor "Extract Method"
4. The refactor will complete without errors or warnings, and the code will become:
...
	public void testMethod() {
		TestClass testClass = null;
		try {
			testClass = extracted();
		} finally {
			if (testClass != null) {
				testClass.doSomething();
			}
		}

	}

	private TestClass extracted() {
		TestClass testClass;
		//Start here
		testClass = testClassFactoryPotentiallyFailing();
		methodPotentiallyThrowingException();
		//End here
		return testClass;
	}
...

The generated code is wrong: in case of exception during "methodPotentiallyThrowingException", the assignment to local variable "testClass" in method "testMethod" will be skipped, so the method "doSomething" (maybe a release of resources...) won't be called.
An error or warning before applying the refactor should be done.