Skip to content

Commit c3cd8b8

Browse files
authored
Improvement to getCustomOptions to prevent premature loading of element info (eclipse-jdt#4779)
Signed-off-by: Rob Stryker <rob@oxbeef.net>
1 parent 1d06512 commit c3cd8b8

File tree

2 files changed

+68
-15
lines changed

2 files changed

+68
-15
lines changed

org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/CompilationUnitTests.java

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,11 @@
3636
import org.eclipse.jdt.core.dom.ASTParser;
3737
import org.eclipse.jdt.core.dom.rewrite.ImportRewrite;
3838
import org.eclipse.jdt.core.formatter.DefaultCodeFormatterConstants;
39+
import org.eclipse.jdt.internal.compiler.env.IElementInfo;
3940
import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
4041
import org.eclipse.jdt.internal.core.Buffer;
4142
import org.eclipse.jdt.internal.core.CompilationUnit;
43+
import org.eclipse.jdt.internal.core.JavaModelManager;
4244
import org.eclipse.jdt.internal.core.util.Util;
4345
import org.eclipse.text.edits.ReplaceEdit;
4446
import org.eclipse.text.edits.TextEdit;
@@ -2882,4 +2884,52 @@ public void java15SwitchExpr(int x){
28822884
assertTrue(methodStatements.size() == 1);
28832885
assertTrue("Should have at least 1 problem", problems.length > 0);
28842886
}
2887+
2888+
/**
2889+
* Test that calling getOptions(true) on a closed ICompilationUnit does NOT
2890+
* cause the element info to be loaded (i.e., does not trigger parsing/opening).
2891+
*
2892+
* Regression test for https://github.com/eclipse-jdt/eclipse.jdt.core/pull/4779
2893+
*
2894+
* Before the fix, getCustomOptions() unconditionally called
2895+
* getCompilationUnitElementInfo(), which triggered openWhenClosed() ->
2896+
* buildStructure() -> parse, even when no setOptions() had ever been called
2897+
* on the CU. After the fix it uses JavaModelManager.getInfo(this) which
2898+
* returns null without opening when the element is not yet cached.
2899+
*/
2900+
public void testGetOptionsDoesNotLoadElementInfo() throws CoreException {
2901+
// Use a CU that is definitely not open (force-close it first).
2902+
ICompilationUnit cu2 = getCompilationUnit("P", "src", "p", "X.java");
2903+
cu2.close(); // ensure info is evicted from the model cache
2904+
2905+
// Precondition: info must be null before we call getOptions.
2906+
IElementInfo infoBefore = JavaModelManager.getJavaModelManager().getInfo(cu2);
2907+
assertNull("Precondition failed: element info should be null before getOptions()", infoBefore);
2908+
2909+
// Call the API under test.
2910+
cu2.getOptions(true);
2911+
2912+
// The fix: info must STILL be null — getOptions must not have opened the CU.
2913+
IElementInfo infoAfter = JavaModelManager.getJavaModelManager().getInfo(cu2);
2914+
assertNull(
2915+
"getOptions(true) should not load the IElementInfo when setOptions() " +
2916+
"has never been called on this CU",
2917+
infoAfter);
2918+
}
2919+
2920+
public void testGetOptionsReturnsCustomOptionsWhenSet() throws CoreException {
2921+
ICompilationUnit cu2 = getCompilationUnit("P", "src", "p", "X.java");
2922+
ICompilationUnit wc = null;
2923+
try {
2924+
wc = cu2.getWorkingCopy(null);
2925+
Map<String, String> opts = new HashMap<>();
2926+
opts.put(JavaCore.COMPILER_SOURCE, "11");
2927+
wc.setOptions(opts);
2928+
2929+
Map<String, String> result = wc.getOptions(false);
2930+
assertEquals("11", result.get(JavaCore.COMPILER_SOURCE));
2931+
} finally {
2932+
if (wc != null) wc.discardWorkingCopy();
2933+
}
2934+
}
28852935
}

org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CompilationUnit.java

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1579,23 +1579,26 @@ public void setOptions(Map<String, String> newOptions) {
15791579
@Override
15801580
public Map<String, String> getCustomOptions() {
15811581
if (this.owner != null) {
1582-
try {
1583-
Map<String, String> customOptions = this.getCompilationUnitElementInfo().getCustomOptions();
1584-
IJavaProject parentProject = getJavaProject();
1585-
Map<String, String> parentOptions = parentProject == null ? JavaCore.getOptions() : parentProject.getOptions(true);
1586-
if (JavaCore.ENABLED.equals(parentOptions.get(JavaCore.COMPILER_PB_ENABLE_PREVIEW_FEATURES)) &&
1587-
CompilerOptions.versionToJdkLevel(parentOptions.getOrDefault(JavaCore.COMPILER_SOURCE, JavaCore.latestSupportedJavaVersion())) < ClassFileConstants.getLatestJDKLevel()) {
1588-
// Disable preview features for older Java releases as it causes the compiler to fail later
1589-
if (customOptions != null) {
1590-
customOptions.put(JavaCore.COMPILER_PB_ENABLE_PREVIEW_FEATURES, JavaCore.DISABLED);
1591-
} else {
1592-
customOptions = Map.of(JavaCore.COMPILER_PB_ENABLE_PREVIEW_FEATURES, JavaCore.DISABLED);
1593-
}
1582+
Map<String, String> customOptions = null;
1583+
JavaModelManager manager = JavaModelManager.getJavaModelManager();
1584+
IElementInfo info = manager.getInfo(this);
1585+
// If the info has not been loaded, then nobody has called setOptions() yet.
1586+
// So no need to load the IElementInfo, which may also parse the cu.
1587+
if (info != null)
1588+
customOptions = ((CompilationUnitElementInfo)info).getCustomOptions();
1589+
1590+
IJavaProject parentProject = getJavaProject();
1591+
Map<String, String> parentOptions = parentProject == null ? JavaCore.getOptions() : parentProject.getOptions(true);
1592+
if (JavaCore.ENABLED.equals(parentOptions.get(JavaCore.COMPILER_PB_ENABLE_PREVIEW_FEATURES)) &&
1593+
CompilerOptions.versionToJdkLevel(parentOptions.getOrDefault(JavaCore.COMPILER_SOURCE, JavaCore.latestSupportedJavaVersion())) < ClassFileConstants.getLatestJDKLevel()) {
1594+
// Disable preview features for older Java releases as it causes the compiler to fail later
1595+
if (customOptions != null) {
1596+
customOptions.put(JavaCore.COMPILER_PB_ENABLE_PREVIEW_FEATURES, JavaCore.DISABLED);
1597+
} else {
1598+
customOptions = Map.of(JavaCore.COMPILER_PB_ENABLE_PREVIEW_FEATURES, JavaCore.DISABLED);
15941599
}
1595-
return customOptions == null ? Collections.emptyMap() : customOptions;
1596-
} catch (JavaModelException e) {
1597-
// do nothing
15981600
}
1601+
return customOptions == null ? Collections.emptyMap() : customOptions;
15991602
}
16001603

16011604
return Collections.emptyMap();

0 commit comments

Comments
 (0)