Skip to content

Unused imports are checked on CI #13216

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

Draft
wants to merge 50 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
db178a0
Enable UnusedImports compiler pass
Akirathan May 14, 2025
11b96e6
Un-ignore all UnusedImportsTest
Akirathan May 14, 2025
2167313
Remove unused imports from Standard.Base.
Akirathan May 14, 2025
384b84a
UsedSymbolCollector handles renamed reexports
Akirathan May 15, 2025
0b5b8cb
More tests for unused imports
Akirathan May 15, 2025
215a116
Create warning only if size of used symbols and size of imported symb…
Akirathan May 15, 2025
324b13a
Merge branch 'develop' into wip/akirathan/12825-remove-unused-imports
Akirathan Jun 3, 2025
5aa348c
Add missing import in Instrumentor
Akirathan Jun 3, 2025
5c50ca7
Add type cast test
Akirathan Jun 3, 2025
a7eceb2
Add missing imports to As_Java_Formatter_Interpreter.enso
Akirathan Jun 3, 2025
5c13909
Reproduce StackOverflowError in unit test
Akirathan Jun 3, 2025
dd10eef
Add another failing unit test
Akirathan Jun 3, 2025
b8733ed
No need for third module in the test
Akirathan Jun 3, 2025
d626690
No need for types in tests
Akirathan Jun 3, 2025
c7c287d
UnusedImports skips synthetic modules
Akirathan Jun 9, 2025
81b7f20
Fix shouldSearchInCurrentModule condition
Akirathan Jun 9, 2025
6339372
Reproduce StackOverflow Error in unit test.
Akirathan Jun 9, 2025
1c4965e
Keep track of which modules we recursed on
Akirathan Jun 9, 2025
83e123e
Add more BindingsMapResolution tests
Akirathan Jun 9, 2025
93ac96c
Add failing UnusedImportsTest
Akirathan Jun 9, 2025
66f437f
Special handling of ResolvedType and ResolvedConstructor for imported…
Akirathan Jun 9, 2025
c9164ca
UsedSymbolCollector first processes meta on IR
Akirathan Jun 9, 2025
58ced82
Add PartiallyAppliedMethod type ascription test
Akirathan Jun 9, 2025
4ea0982
Add back some unnecessary imports of extension methods
Akirathan Jun 10, 2025
1ada18c
Remove unused imports from Standard.Table
Akirathan Jun 10, 2025
fda738a
Add back some unnecessary imports of extension methods
Akirathan Jun 10, 2025
73379c8
Remove unused imports from Standard.AWS
Akirathan Jun 10, 2025
3f9b173
Remove unused imports from Standard.Database
Akirathan Jun 10, 2025
d1f677c
Remove unused imports from Standard.Google_Api
Akirathan Jun 10, 2025
b0af512
Remove unused imports from Standard.Microsoft
Akirathan Jun 10, 2025
ec58ccb
Remove unused imports from Standard.Snowflake
Akirathan Jun 10, 2025
af3a3e5
Remove unused imports from Standard.Tableau
Akirathan Jun 10, 2025
7f30382
Remove unused imports from Standard.Test
Akirathan Jun 10, 2025
db63a7b
Remove unused imports from Standard.Visualization
Akirathan Jun 10, 2025
48d071b
Add back some unnecessary imports
Akirathan Jun 11, 2025
c69345f
Merge branch 'develop' into wip/akirathan/12825-remove-unused-imports
Akirathan Jun 11, 2025
5ab4377
Remove duplicated imports - fix after merge
Akirathan Jun 11, 2025
d3f50d2
Remove unused imports from Reporting_Stream_Decoder_Helper
Akirathan Jun 11, 2025
1e4dacd
Remove duplicated imports - fix after merge
Akirathan Jun 11, 2025
24e2be9
Add export State to Runtime.
Akirathan Jun 12, 2025
8a798f3
Add synthetic submodule test
Akirathan Jun 12, 2025
612441d
Merge branch 'develop' into wip/akirathan/12825-remove-unused-imports
Akirathan Jun 12, 2025
efae13e
Fix false positive in `from Standard.Base.Runtime import State`.
Akirathan Jun 12, 2025
de28d6e
No need to export submodule
Akirathan Jun 12, 2025
36d87d0
fmt
Akirathan Jun 12, 2025
1ba6a69
Fixes after merge
Akirathan Jun 12, 2025
0be04ad
UnusedImports compiler pass runs only when static analysis is enabled
Akirathan Jun 12, 2025
e822b25
Remove unused imports
Akirathan Jun 12, 2025
ae569f5
Ensure static analysis is enabled in runtime-compiler/test
Akirathan Jun 13, 2025
cee5db1
Merge branch 'develop' into wip/akirathan/12825-remove-unused-imports
Akirathan Jun 13, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion engine/runner/src/main/java/org/enso/runner/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,8 @@ private void createNew(
* global cache
* @param shouldUseIrCaches whether or not IR caches should be used.
* @param disablePrivateCheck whether or not the private check should be disabled
* @param enableStaticAnalysis whether or not static type checking should be enabled
* @param enableStaticAnalysis whether or not static type checking, and other static analysis,
* should be enabled
* @param treatWarningsAsErrors whether or not warnings should be treated as errors
* @param logLevel the logging level
* @param logMasking whether or not log masking is enabled
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
* @param autoParallelismEnabled whether or not automatic parallelism detection is enabled.
* @param warningsEnabled whether or not warnings are enabled
* @param privateCheckEnabled whether or not private keyword is enabled
* @param staticTypeInferenceEnabled whether or not type inference is enabled
* @param staticAnalysisEnabled whether or not type inference, and other static analysis, is enabled
* @param treatWarningsAsErrors If warnings should be treated as errors.
* @param dumpModuleIR identification (name) of a module to dump
* @param isStrictErrors if true, presence of any Error in IR will result in an exception
Expand All @@ -22,7 +22,7 @@ public record CompilerConfig(
boolean autoParallelismEnabled,
boolean warningsEnabled,
boolean privateCheckEnabled,
boolean staticTypeInferenceEnabled,
boolean staticAnalysisEnabled,
boolean treatWarningsAsErrors,
Option<String> dumpModuleIR,
boolean isStrictErrors,
Expand All @@ -42,7 +42,7 @@ public static final class Builder {
private boolean autoParallelismEnabled = false;
private boolean warningsEnabled = true;
private boolean privateCheckEnabled = true;
private boolean staticTypeInferenceEnabled = false;
private boolean staticAnalysisEnabled = false;
private boolean treatWarningsAsErrors = false;
private Option<String> dumpModuleIR = Option.empty();
private boolean isStrictErrors = false;
Expand All @@ -66,8 +66,8 @@ public Builder privateCheckEnabled(boolean privateCheckEnabled) {
return this;
}

public Builder staticTypeInferenceEnabled(boolean staticTypeInferenceEnabled) {
this.staticTypeInferenceEnabled = staticTypeInferenceEnabled;
public Builder staticAnalysisEnabled(boolean staticAnalysisEnabled) {
this.staticAnalysisEnabled = staticAnalysisEnabled;
return this;
}

Expand Down Expand Up @@ -111,7 +111,7 @@ public CompilerConfig build() {
autoParallelismEnabled,
warningsEnabled,
privateCheckEnabled,
staticTypeInferenceEnabled,
staticAnalysisEnabled,
treatWarningsAsErrors,
dumpModuleIR,
isStrictErrors,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package org.enso.compiler.pass.lint.unusedimports;

import static org.enso.scala.wrapper.ScalaConversions.asJava;

import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import org.enso.compiler.context.InlineContext;
import org.enso.compiler.context.ModuleContext;
import org.enso.compiler.core.CompilerError;
Expand All @@ -17,7 +20,10 @@
import org.enso.compiler.core.ir.module.scope.Import;
import org.enso.compiler.core.ir.module.scope.imports.Polyglot;
import org.enso.compiler.data.BindingsMap;
import org.enso.compiler.data.BindingsMap.ResolvedConstructor;
import org.enso.compiler.data.BindingsMap.ResolvedModule;
import org.enso.compiler.data.BindingsMap.ResolvedName;
import org.enso.compiler.data.BindingsMap.ResolvedType;
import org.enso.compiler.pass.IRPass;
import org.enso.compiler.pass.IRProcessingPass;
import org.enso.compiler.pass.analyse.AmbiguousImportsAnalysis;
Expand Down Expand Up @@ -85,19 +91,15 @@ public Expression runExpression(Expression ir, InlineContext inlineContext) {

@Override
public Module runModule(Module ir, ModuleContext moduleContext) {
if (moduleContext.isSynthetic()) {
return ir;
}
var bm = moduleContext.bindingsAnalysis();
var usedSymbols = UsedSymbolsCollector.collect(ir, bm);
for (var imp : CollectionConverters.asJava(ir.imports())) {
if (!shouldImportBeSkipped(imp)) {
if (imp instanceof Import.Module impMod && impMod.onlyNames().isDefined()) {
var importedSymbols = importedSymbols(imp, bm);
var usedSymbolsForImp = usedSymbols.getUsedSymbolsForImport(imp);
var diff = new HashSet<>(importedSymbols);
diff.removeAll(usedSymbolsForImp);
if (!diff.isEmpty()) {
var warn = createWarning(imp, diff);
imp.getDiagnostics().add(warn);
}
handleOnlyNamesImport(impMod, bm, usedSymbols);
} else {
var usedSymbolsForImp = usedSymbols.getUsedSymbolsForImport(imp);
if (usedSymbolsForImp.isEmpty()) {
Expand All @@ -110,8 +112,83 @@ public Module runModule(Module ir, ModuleContext moduleContext) {
return ir;
}

/**
* Handles import of type {@code from ... import Symbol_1, Symbol_2}. This is the only
* "interesting" import for this whole pass, as we have to determine for every symbol if it is
* actually used or not.
*/
private void handleOnlyNamesImport(
Import.Module imp, BindingsMap bindingsMap, UsedSymbols usedSymbols) {
assert imp.onlyNames().isDefined();
var importedSymbols = importedSymbols(imp, bindingsMap);
var usedSymbolsForImp = usedSymbols.getUsedSymbolsForImport(imp);
var unusedSymbols =
importedSymbols.stream().filter(impSym -> !isImportedSymbolUsed(impSym, usedSymbolsForImp));
var unusedSymbolsNames =
unusedSymbols.map(ResolvedName::qualifiedName).collect(Collectors.toUnmodifiableSet());
if (!unusedSymbolsNames.isEmpty()) {
var warn = createWarning(imp, unusedSymbolsNames);
imp.getDiagnostics().add(warn);
}
}

/**
* Returns true if the given {@code importedSymbol} is among used symbols.
*
* <h4>Example</h4>
*
* If the imported symbol is a type, and it its fully qualified name is not in {@code
* usedSymbolsForImp}, all it's constructors are checked for the presence in {@code
* usedSymbolsForImp}. So for example if imported symbol is a type {@code local.Proj.Module.T},
* and its constructor {@code local.Proj.Module.T.Cons} is in used symbols, the type is considered
* used.
*
* @param importedSymbol Symbol import from the import IR.
* <p>For example, in {@code from project.Module import Type}, the imported symbol will be
* {@link ResolvedType}.
* @param usedSymbolsForImp Set of used symbols for a particular import IR. As gathered by {@link
* UsedSymbolsCollector}.
*/
private boolean isImportedSymbolUsed(
ResolvedName importedSymbol, Set<QualifiedName> usedSymbolsForImp) {
var importedSymbolName = importedSymbol.qualifiedName();
if (usedSymbolsForImp.contains(importedSymbolName)) {
return true;
}
return switch (importedSymbol) {
case ResolvedConstructor cons -> {
var typeName = cons.tpe().qualifiedName();
var isTypeUsed = usedSymbolsForImp.contains(typeName);
yield isTypeUsed;
}
// If any of the Type's constructor is used, the whole type is used.
case ResolvedType type -> {
var constructors = asJava(type.tp().members());
var typeName = type.qualifiedName();
var consNames = constructors.stream().map(cons -> typeName.createChild(cons.name()));
var isInUsedSymbols = consNames.anyMatch(usedSymbolsForImp::contains);
yield isInUsedSymbols;
}
case ResolvedModule module -> {
for (var usedSymbol : usedSymbolsForImp) {
// All but the last item
var prefix = usedSymbol.path().mkString(".");
if (module.qualifiedName().toString().equals(prefix)) {
var res = module.findExportedSymbolsFor(usedSymbol.item());
var symbolInModule = !res.isEmpty();
if (symbolInModule) {
yield true;
}
}
}
yield false;
}
default -> false;
};
}

/** Returns set of all imported symbol by the given import statement. */
private Set<QualifiedName> importedSymbols(Import impIr, BindingsMap bindingsMap) {
private Set<ResolvedName> importedSymbols(Import impIr, BindingsMap bindingsMap) {
var resolvedImp = findResolvedImport(impIr, bindingsMap);
if (resolvedImp == null) {
var errMsg = new StringBuilder();
Expand All @@ -132,7 +209,7 @@ private Set<QualifiedName> importedSymbols(Import impIr, BindingsMap bindingsMap
return new HashSet<>(symbols);
}

private static List<QualifiedName> importedSymbols(BindingsMap.ResolvedImport resolvedImport) {
private static List<ResolvedName> importedSymbols(BindingsMap.ResolvedImport resolvedImport) {
var impDef = resolvedImport.importDef();
if (impDef.onlyNames().isDefined()) {
var targets = resolvedImport.targets();
Expand All @@ -146,20 +223,20 @@ private static List<QualifiedName> importedSymbols(BindingsMap.ResolvedImport re
}
var target = targets.head();
var names = impDef.onlyNames().get().map(Literal::name);
var resolvedNames = new ArrayList<QualifiedName>();
var resolvedNames = new ArrayList<ResolvedName>();
names.foreach(
name -> {
var expSymbols = target.findExportedSymbolsFor(name);
expSymbols.foreach(
expSymbol -> {
resolvedNames.add(expSymbol.qualifiedName());
resolvedNames.add(expSymbol);
return null;
});
return null;
});
return resolvedNames;
} else {
var names = resolvedImport.targets().map(ResolvedName::qualifiedName);
var names = resolvedImport.targets().map(target -> (ResolvedName) target);
return CollectionConverters.asJava(names);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
import org.enso.compiler.core.ir.module.scope.Export;
import org.enso.compiler.core.ir.module.scope.Import;
import org.enso.compiler.data.BindingsMap;
import org.enso.compiler.data.BindingsMap.ImportTarget;
import org.enso.compiler.data.BindingsMap.Resolution;
import org.enso.compiler.data.BindingsMap.ResolvedModule;
import org.enso.compiler.data.BindingsMap.ResolvedName;
import org.enso.compiler.pass.lint.unusedimports.UsedSymbols.Builder;
import org.enso.compiler.pass.resolve.GenericAnnotations$;
Expand Down Expand Up @@ -73,6 +75,22 @@ private void collectFromRoot(IR root) {
irsToProcess.add(root);
while (!irsToProcess.isEmpty()) {
var ir = irsToProcess.removeFirst();

addUsedSymbolForResolution(getGlobalNamesMeta(ir));
addUsedSymbolForResolution(getMethodDefinitionsMeta(ir));
addUsedSymbolForResolution(getTypeNameMeta(ir));
addUsedSymbolForResolution(getPatternsMeta(ir));
var typeSig = getTypeSignatureMeta(ir);
if (typeSig != null) {
var sig = typeSig.signature();
irsToProcess.addLast(sig);
}
var anotMeta = getGenericAnnotationMeta(ir);
if (anotMeta != null) {
var annotations = asJava(anotMeta.annotations());
irsToProcess.addAll(annotations);
}

// Application.Prefix (method calls) are handled specifically. GlobalNames pass assigns
// resolution to the first synthetic self argument.
if (ir instanceof Application.Prefix app
Expand Down Expand Up @@ -104,30 +122,12 @@ private void collectFromRoot(IR root) {
// Add all the children except for the first argument
asJava(app.arguments()).stream().skip(1).forEach(irsToProcess::addLast);
irsToProcess.addLast(app.function());
} else {
irsToProcess.addAll(asJava(ir.children()));
continue;
}
} else {
irsToProcess.addAll(asJava(ir.children()));
}
} else {
addUsedSymbolForResolution(getGlobalNamesMeta(ir));
addUsedSymbolForResolution(getMethodDefinitionsMeta(ir));
addUsedSymbolForResolution(getTypeNameMeta(ir));
addUsedSymbolForResolution(getPatternsMeta(ir));
var typeSig = getTypeSignatureMeta(ir);
if (typeSig != null) {
var sig = typeSig.signature();
irsToProcess.addLast(sig);
}
var anotMeta = getGenericAnnotationMeta(ir);
if (anotMeta != null) {
var annotations = asJava(anotMeta.annotations());
irsToProcess.addAll(annotations);
}
var children = asJava(ir.children());
irsToProcess.addAll(children);
}
var children = asJava(ir.children());
irsToProcess.addAll(children);
}
}

Expand Down Expand Up @@ -178,12 +178,21 @@ private List<Import.Module> findImportIRs(
.targets()
.find(
target -> {
var resolvedNames = target.findExportedSymbolsFor(targetSymbolName.item());
var targetNameMatches = target.qualifiedName().equals(targetSymbolName);
var exportsSymbol = !resolvedNames.isEmpty();
if (targetNameMatches || exportsSymbol) {
if (target.qualifiedName().equals(targetSymbolName)) {
return true;
}
if (exportsSymbol(target, targetSymbolName.item())) {
return true;
}

if (target instanceof ResolvedModule) {
var qualifiedRes =
bindingsMap.resolveQualifiedName(targetSymbolName.fullPath());
if (qualifiedRes.toOption().isDefined()) {
return true;
}
}

var suffix = dropPrefix(targetModName, targetSymbolName);
if (suffix.size() > 1) {
// We are trying to resolve a qualified name within a module of size greater
Expand Down Expand Up @@ -216,6 +225,11 @@ private List<Import.Module> findImportIRs(
return importDefs;
}

private static boolean exportsSymbol(ImportTarget target, String symbol) {
var resolvedNames = target.findExportedSymbolsFor(symbol);
return !resolvedNames.isEmpty();
}

@SuppressWarnings("unchecked")
private static List<String> dropPrefix(QualifiedName prefix, QualifiedName name) {
var namePath = name.fullPath();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import org.enso.compiler.pass.analyse.types.{
TypeInferenceSignatures
}
import org.enso.compiler.pass.desugar._
import org.enso.compiler.pass.lint.unusedimports.UnusedImports
import org.enso.compiler.pass.lint.{
ModuleNameConflicts,
NoSelfInStatic,
Expand Down Expand Up @@ -103,16 +104,17 @@ class Passes(config: CompilerConfig) {
Nil
} else {
List(UnusedBindings, NoSelfInStatic)
}) ++ (if (config.staticTypeInferenceEnabled) {
}) ++ (if (config.staticAnalysisEnabled) {
List(
TypeInferenceSignatures.INSTANCE,
StaticModuleScopeAnalysis.INSTANCE
StaticModuleScopeAnalysis.INSTANCE,
UnusedImports.INSTANCE
)
} else Nil)
)

val typeInferenceFinalPasses = new PassGroup(
if (config.staticTypeInferenceEnabled) {
if (config.staticAnalysisEnabled) {
List(
TypeInferencePropagation.INSTANCE
)
Expand Down
Loading
Loading