Skip to content

Commit

Permalink
OPTION_JdtDebugCompileMode has surprising effects on imports
Browse files Browse the repository at this point in the history
Replace OPTION_JdtDebugCompileMode with
OPTION_IgnoreUnnamedModuleForSplitPackage, to avoid unexpected effects
from the first.

Signed-off-by: Simeon Andreev <simeon.danailov.andreev@gmail.com>
  • Loading branch information
trancexpress committed Jun 12, 2023
1 parent 7e1df7e commit a122085
Show file tree
Hide file tree
Showing 7 changed files with 11 additions and 76 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import org.eclipse.jdt.internal.compiler.ASTVisitor;
import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
import org.eclipse.jdt.internal.compiler.lookup.*;

public class ImportReference extends ASTNode {
Expand Down Expand Up @@ -71,11 +70,7 @@ public void checkPackageConflict(CompilationUnitScope scope) {
declaringMods.add(incarnation.enclosingModule);
}
if (!declaringMods.isEmpty()) {
CompilerOptions compilerOptions = scope.compilerOptions();
boolean inJdtDebugCompileMode = compilerOptions.enableJdtDebugCompileMode;
if (!inJdtDebugCompileMode) {
scope.problemReporter().conflictingPackagesFromOtherModules(this, declaringMods);
}
scope.problemReporter().conflictingPackagesFromOtherModules(this, declaringMods);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.eclipse.jdt.internal.compiler.ClassFile;
import org.eclipse.jdt.internal.compiler.CompilationResult;
import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
import org.eclipse.jdt.internal.compiler.impl.ReferenceContext;
import org.eclipse.jdt.internal.compiler.lookup.CompilationUnitScope;
import org.eclipse.jdt.internal.compiler.lookup.ExtraCompilerModifiers;
Expand Down Expand Up @@ -347,11 +346,7 @@ private void analyseOneDependency(RequiresStatement requiresStat, ModuleBinding
for (PlainPackageBinding pack : requiredModule.getExports()) {
Set<ModuleBinding> mods = pack2mods.get(String.valueOf(pack.readableName()));
if (mods != null && mods.size() > 1) {
CompilerOptions compilerOptions = skope.compilerOptions();
boolean inJdtDebugCompileMode = compilerOptions.enableJdtDebugCompileMode;
if (!inJdtDebugCompileMode) {
skope.problemReporter().conflictingPackagesFromModules(pack, mods, requiresStat.sourceStart, requiresStat.sourceEnd);
}
skope.problemReporter().conflictingPackagesFromModules(pack, mods, requiresStat.sourceStart, requiresStat.sourceEnd);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

import org.eclipse.jdt.core.compiler.CharOperation;
import org.eclipse.jdt.internal.compiler.ASTVisitor;
import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
import org.eclipse.jdt.internal.compiler.lookup.*;
import org.eclipse.jdt.internal.compiler.problem.AbortCompilation;

Expand Down Expand Up @@ -126,14 +125,10 @@ protected TypeBinding getTypeBinding(Scope scope) {
if (packageBinding != null) {
PackageBinding uniquePackage = packageBinding.getVisibleFor(scope.module(), false);
if (uniquePackage instanceof SplitPackageBinding) {
CompilerOptions compilerOptions = scope.compilerOptions();
boolean inJdtDebugCompileMode = compilerOptions.enableJdtDebugCompileMode;
if (!inJdtDebugCompileMode) {
SplitPackageBinding splitPackage = (SplitPackageBinding) uniquePackage;
scope.problemReporter().conflictingPackagesFromModules(splitPackage, scope.module(), this.sourceStart, (int)this.sourcePositions[typeStart-1]);
this.resolvedType = new ProblemReferenceBinding(this.tokens, null, ProblemReasons.Ambiguous);
return null;
}
SplitPackageBinding splitPackage = (SplitPackageBinding) uniquePackage;
scope.problemReporter().conflictingPackagesFromModules(splitPackage, scope.module(), this.sourceStart, (int)this.sourcePositions[typeStart-1]);
this.resolvedType = new ProblemReferenceBinding(this.tokens, null, ProblemReasons.Ambiguous);
return null;
}
}
rejectAnnotationsOnPackageQualifiers(scope, packageBinding);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,6 @@ public class CompilerOptions {

public static final String OPTION_ReportSuppressWarningNotFullyAnalysed = "org.eclipse.jdt.core.compiler.problem.suppressWarningsNotFullyAnalysed"; //$NON-NLS-1$

// Internally used option to allow debug framework compile evaluation snippets in context of modules, see bug 543604
public static final String OPTION_JdtDebugCompileMode = "org.eclipse.jdt.internal.debug.compile.mode"; //$NON-NLS-1$

public static final String OPTION_IgnoreUnnamedModuleForSplitPackage = "org.eclipse.jdt.core.compiler.ignoreUnnamedModuleForSplitPackage"; //$NON-NLS-1$

/**
Expand Down Expand Up @@ -546,9 +543,6 @@ public class CompilerOptions {
/** Master flag to enabled/disable all preview features */
public boolean enablePreviewFeatures;

/** Enable a less restrictive compile mode for JDT debug. */
public boolean enableJdtDebugCompileMode;

/** Should the compiler ignore the unnamed module when a package is defined in both a named module and the unnamed module? */
public boolean ignoreUnnamedModuleForSplitPackage;

Expand Down Expand Up @@ -1600,7 +1594,6 @@ protected void resetDefaults() {
this.complainOnUninternedIdentityComparison = false;
this.enablePreviewFeatures = false;

this.enableJdtDebugCompileMode = false;
this.ignoreUnnamedModuleForSplitPackage = false;
}

Expand Down Expand Up @@ -2128,14 +2121,6 @@ && getSeverity(ExplicitlyClosedAutoCloseable) == ProblemSeverities.Ignore) {
if ((optionValue = optionsMap.get(OPTION_ReportSuppressWarningNotFullyAnalysed)) != null)
updateSeverity(SuppressWarningsNotAnalysed, optionValue);

if ((optionValue = optionsMap.get(OPTION_JdtDebugCompileMode)) != null) {
if (ENABLED.equals(optionValue)) {
this.enableJdtDebugCompileMode = true;
} else if (DISABLED.equals(optionValue)) {
this.enableJdtDebugCompileMode = false;
}
}

if ((optionValue = optionsMap.get(OPTION_IgnoreUnnamedModuleForSplitPackage)) != null) {
if (ENABLED.equals(optionValue)) {
this.ignoreUnnamedModuleForSplitPackage = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,6 @@ void faultInImports() {
this.importPtr = 1;

CompilerOptions compilerOptions = compilerOptions();
boolean inJdtDebugCompileMode = compilerOptions.enableJdtDebugCompileMode;

// keep static imports with normal imports until there is a reason to split them up
// on demand imports continue to be packages & types. need to check on demand type imports for fields/methods
Expand All @@ -464,7 +463,7 @@ void faultInImports() {
}
if (importBinding instanceof PackageBinding) {
PackageBinding uniquePackage = ((PackageBinding)importBinding).getVisibleFor(module(), false);
if (uniquePackage instanceof SplitPackageBinding && !inJdtDebugCompileMode) {
if (uniquePackage instanceof SplitPackageBinding) {
SplitPackageBinding splitPackage = (SplitPackageBinding) uniquePackage;
problemReporter().conflictingPackagesFromModules(splitPackage, module(), importReference.sourceStart, importReference.sourceEnd);
continue nextImport;
Expand All @@ -477,7 +476,7 @@ void faultInImports() {
recordImportBinding(new ImportBinding(compoundName, true, importBinding, importReference));
} else {
Binding importBinding = findSingleImport(compoundName, Binding.TYPE | Binding.FIELD | Binding.METHOD, importReference.isStatic());
if (importBinding instanceof SplitPackageBinding && !inJdtDebugCompileMode) {
if (importBinding instanceof SplitPackageBinding) {
SplitPackageBinding splitPackage = (SplitPackageBinding) importBinding;
int sourceEnd = (int)(importReference.sourcePositions[splitPackage.compoundName.length-1] & 0xFFFF);
problemReporter().conflictingPackagesFromModules((SplitPackageBinding) importBinding, module(), importReference.sourceStart, sourceEnd);
Expand Down Expand Up @@ -508,7 +507,7 @@ void faultInImports() {
importedPackage = (PackageBinding) findImport(importedPackage.compoundName, false, true);
if (importedPackage != null)
importedPackage = importedPackage.getVisibleFor(module(), true);
if (importedPackage instanceof SplitPackageBinding && !inJdtDebugCompileMode) {
if (importedPackage instanceof SplitPackageBinding) {
SplitPackageBinding splitPackage = (SplitPackageBinding) importedPackage;
int sourceEnd = (int) importReference.sourcePositions[splitPackage.compoundName.length-1];
problemReporter().conflictingPackagesFromModules(splitPackage, module(), importReference.sourceStart, sourceEnd);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1498,7 +1498,7 @@ private boolean flaggedJavaBaseTypeErrors(ReferenceBinding result, char[][] comp
ModuleBinding visibleModule = currentPack != null ? currentPack.enclosingModule : null;
if (visibleModule != null && visibleModule != javaBaseModule()) {
// A type from java.base is not visible
if (!this.globalOptions.enableJdtDebugCompileMode) {
if (!this.globalOptions.ignoreUnnamedModuleForSplitPackage) {
this.problemReporter.conflictingPackageInModules(compoundName, this.root.unitBeingCompleted, this.missingClassFileLocation,
readableName, TypeConstants.JAVA_BASE, visibleModule.readableName());
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3407,7 +3407,7 @@ public void test_no_conflicting_packages_for_debugger_global() throws CoreExcept
Hashtable<String, String> javaCoreOptions = JavaCore.getOptions();
try {
Hashtable<String, String> newOptions=new Hashtable<>(javaCoreOptions);
newOptions.put(CompilerOptions.OPTION_JdtDebugCompileMode, JavaCore.ENABLED);
newOptions.put(CompilerOptions.OPTION_IgnoreUnnamedModuleForSplitPackage, JavaCore.ENABLED);
JavaCore.setOptions(newOptions);
String[] sources = new String[] {
"src/java/util/Map___.java",
Expand All @@ -3425,46 +3425,12 @@ public void test_no_conflicting_packages_for_debugger_global() throws CoreExcept
IJavaProject p1= setupModuleProject("debugger_project", sources, new IClasspathEntry[]{dep});
p1.getProject().getWorkspace().build(IncrementalProjectBuilder.FULL_BUILD, null);
assertNoErrors();

assertNull("Option should not be stored", JavaCore.getOption(CompilerOptions.OPTION_JdtDebugCompileMode));
} finally {
deleteProject("debugger_project");
JavaCore.setOptions(javaCoreOptions);
}
}

// test that the special OPTION_JdtDebugCompileMode cannot be persisted on a project
public void test_no_conflicting_packages_for_debugger_project() throws CoreException {
try {
String[] sources = new String[] {
"src/java/util/Map___.java",
"package java.util;\n" +
"abstract class Map___ implements java.util.Map {\n" +
" Map___() {\n" +
" super();\n" +
" }\n" +
" Object[] ___run() throws Throwable {\n" +
" return entrySet().toArray();\n" +
" }\n" +
"}"
};
IClasspathEntry dep = JavaCore.newContainerEntry(new Path(JavaCore.MODULE_PATH_CONTAINER_ID));
IJavaProject p1= setupModuleProject("debugger_project", sources, new IClasspathEntry[]{dep});
p1.setOption(CompilerOptions.OPTION_JdtDebugCompileMode, JavaCore.ENABLED);
assertNull("Option should not be stored", p1.getOption(CompilerOptions.OPTION_JdtDebugCompileMode, false));
p1.getProject().getWorkspace().build(IncrementalProjectBuilder.FULL_BUILD, null);
IMarker[] markers = p1.getProject().findMarkers(null, true, IResource.DEPTH_INFINITE);
sortMarkers(markers);
assertMarkers("Unexpected markers",
"The package java.util conflicts with a package accessible from another module: java.base\n" +
"The package java.util is accessible from more than one module: <unnamed>, java.base\n" +
"The method entrySet() is undefined for the type Map___",
markers);
} finally {
deleteProject("debugger_project");
}
}

// test that a package declared in a module conflicts with an accessible package
// of the same name declared in another required module
public void test_conflicting_packages_declaredvsaccessible() throws CoreException {
Expand Down

0 comments on commit a122085

Please sign in to comment.