Cleanup import resolution logic

src/compile/inference/imports.ts

@@ -1,6 +1,7 @@
 import fse from "fs-extra";
-import { dirname, isAbsolute, join, normalize } from "path";
+import { dirname, normalize } from "path";
 import { CompileInferenceError, ImportResolver, Remapping } from "..";
+import { assert } from "../..";
 import {
     AnyFileLevelNode,
     FileLevelNodeKind,
@@ -22,12 +23,6 @@ function applyRemappings(remappings: Remapping[], path: string): string {
     return path;
 }
 
-export function normalizeImportPath(path: string): string {
-    const normalized = normalize(path);
-
-    return isAbsolute(normalized) ? normalized : "./" + normalized;
-}
-
 /**
  * Check that `path` starts with a relative prefix "." or "./".  Note that we
  * don't use `!path.isAbsolute(path)` on purpose.  Otherwise it might consider
@@ -38,66 +33,92 @@ function isPathWithRelativePrefix(path: string): boolean {
 }
 
 /**
- * Given the `importer` unit path, and the path `imported` of an import directive
- * inside the unit, compute the real path of the imported unit.
+ * Normalize a relative import path as described in
+ * https://docs.soliditylang.org/en/v0.8.8/path-resolution.html#relative-imports
+ * @param importer - source unit name of importing unit
+ * @param imported - path of import directive
+ */
+function normalizeRelativeImportPath(importer: string, imported: string): string {
+    imported = normalize(imported);
+    let prefix = dirname(importer);
+    const importedSegments = imported.split("/").filter((s) => s != "");
+    let strippedSegments = 0;
+
+    while (
+        strippedSegments < importedSegments.length &&
+        importedSegments[strippedSegments] === ".."
+    ) {
+        prefix = dirname(prefix);
+        strippedSegments++;
+    }
+
+    // According to https://docs.soliditylang.org/en/v0.8.8/path-resolution.html#relative-imports when prefix
+    // is empty there is no leading "./". However `dirname` always returns non-empty prefixes.
+    // Handle this edge case.
+    if (prefix === "." && !importer.startsWith(".")) {
+        prefix = "";
+    }
+
+    assert(prefix === "" || !prefix.endsWith("/"), `Invalid prefix ${prefix}`);
+    const suffix = importedSegments.slice(strippedSegments).join("/");
+    return prefix === "" ? suffix : prefix + "/" + suffix;
+}
+
+/**
+ * Given the `importer` source unit path, and the path `imported` of an import
+ * directive compute the expected **source unit name** of the imported file.
+ * This is the name the compiler will look for in its "VFS" as defined starting
+ * here:
+ *
+ * https://docs.soliditylang.org/en/v0.8.8/path-resolution.html
  *
- * This takes into account remappings, relative and absolute paths.
- * Note: The returned path is not neccessarily absolute!
+ * This takes into account relative and absolute paths and remappings.
  */
-function computeRealPath(
-    importer: string | undefined,
+function computeSourceUnitName(
+    importerSourceUnit: string,
     imported: string,
     remappings: Remapping[]
 ): string {
-    let result = applyRemappings(remappings, imported);
-
-    if (importer !== undefined && isPathWithRelativePrefix(result)) {
-        const importingFileDir = dirname(importer);
-
-        result = normalizeImportPath(join(importingFileDir, result));
+    if (isPathWithRelativePrefix(imported)) {
+        return normalizeRelativeImportPath(importerSourceUnit, imported);
     }
 
-    return result;
+    return applyRemappings(remappings, imported);
 }
 
 /**
- * Given a partial map `files` from file names to file contents, a list of
- * `remappings` and a list of `ImportResolver`s `resolvers`, find and return all
- * ADDITIONAL files that are imported from the starting set `files` but are
- * missing in `files`. The return value is also a map from file names to file
- * contents.
+ * Given a partial map `files` from **source unit names** to file contents, a list of
+ * `remappings` and a list of `ImportResolver`s - `resolvers`, find all
+ * files that are imported from the starting set `files` but are
+ * **missing** in `files` and add them into the files map.
  */
 export function findAllFiles(
     files: Map<string, string>,
     remappings: Remapping[],
     resolvers: ImportResolver[]
-): Map<string, string> {
-    const queue: Array<[string | undefined, string]> = [];
-
-    for (const fileName of files.keys()) {
-        queue.push([undefined, fileName]);
-    }
-
+): void {
+    // Queue of source unit names to process
+    const queue: string[] = [...files.keys()];
     const visited = new Set<string>();
-    const result = new Map<string, string>();
 
     while (queue.length > 0) {
-        const [importer, imported] = queue.pop() as [string | undefined, string];
-
-        const realPath = computeRealPath(importer, imported, remappings);
+        const sourceUnitName = queue.pop() as string;
 
         /**
-         * Skip already processed imports
+         * Skip already processed units
          */
-        if (visited.has(realPath)) {
+        if (visited.has(sourceUnitName)) {
             continue;
         }
 
-        let content = files.get(realPath);
+        visited.add(sourceUnitName);
 
+        let content = files.get(sourceUnitName);
+
+        // Missing contents - try and fill them in from the resolvers
         if (content === undefined) {
             for (const resolver of resolvers) {
-                const resolvedPath = resolver.resolve(realPath);
+                const resolvedPath = resolver.resolve(sourceUnitName);
 
                 if (resolvedPath !== undefined) {
                     content = fse.readFileSync(resolvedPath, { encoding: "utf-8" });
@@ -107,16 +128,12 @@ export function findAllFiles(
             }
 
             if (content === undefined) {
-                throw new CompileInferenceError(
-                    `Couldn't find ${realPath} imported from ${importer}`
-                );
+                throw new CompileInferenceError(`Couldn't find ${sourceUnitName}`);
             }
 
-            result.set(isPathWithRelativePrefix(imported) ? realPath : imported, content);
+            files.set(sourceUnitName, content);
         }
 
-        visited.add(realPath);
-
         let flds: AnyFileLevelNode[];
 
         try {
@@ -128,7 +145,7 @@ export function findAllFiles(
                 const length = end - start;
 
                 throw new CompileInferenceError(
-                    `Failed parsing imports at ${realPath}:${start}:${length} - ${e.message}`
+                    `Failed parsing imports at ${sourceUnitName}:${start}:${length} - ${e.message}`
                 );
             }
 
@@ -137,10 +154,8 @@ export function findAllFiles(
 
         for (const fld of flds) {
             if (fld.kind === FileLevelNodeKind.Import) {
-                queue.push([realPath, fld.path]);
+                queue.push(computeSourceUnitName(sourceUnitName, fld.path, remappings));
             }
         }
     }
-
-    return result;
 }

src/compile/input.ts

@@ -3,7 +3,11 @@ import { CompilationOutput, CompilerKind } from "./constants";
 
 export interface PartialSolcInput {
     language: "Solidity";
-    settings: { outputSelection: any; [otherKeys: string]: any };
+    settings: {
+        outputSelection: any;
+        remappings: string[];
+        [otherKeys: string]: any;
+    };
     [otherKeys: string]: any;
 }
 
@@ -39,6 +43,7 @@ function mergeCompilerSettings<T extends Solc04Input | Solc05Input>(input: T, se
  */
 export function createCompilerInput(
     files: Map<string, string>,
+    remappings: string[],
     version: string,
     kind: CompilerKind,
     output: CompilationOutput[],
@@ -64,6 +69,7 @@ export function createCompilerInput(
     const partialInp: PartialSolcInput = {
         language: "Solidity",
         settings: {
+            remappings,
             outputSelection: {
                 "*": {
                     "*": contractOutput,

src/compile/utils.ts

@@ -9,7 +9,7 @@ import {
 } from "./compiler_selection";
 import { CompilationOutput, CompilerKind } from "./constants";
 import { Remapping } from "./import_resolver";
-import { findAllFiles, normalizeImportPath } from "./inference";
+import { findAllFiles } from "./inference";
 import { createCompilerInput } from "./input";
 import { getNativeCompilerForVersion, getWasmCompilerForVersion } from "./kinds";
 import { getCompilerPrefixForOs } from "./kinds/md";
@@ -85,11 +85,7 @@ export function resolveFiles(
     resolvers: ImportResolver[]
 ): void {
     const parsedRemapping = parsePathRemapping(remapping);
-    const additionalFiles = findAllFiles(files, parsedRemapping, resolvers);
-
-    for (const [fileName, source] of additionalFiles) {
-        files.set(fileName, source);
-    }
+    findAllFiles(files, parsedRemapping, resolvers);
 }
 
 function fillFilesFromSources(
@@ -120,13 +116,15 @@ function getCompilerVersionStrategy(
 
 export async function compile(
     files: Map<string, string>,
+    remapping: string[],
     version: string,
     compilationOutput: CompilationOutput[] = [CompilationOutput.ALL],
     compilerSettings?: any,
     kind = CompilerKind.WASM
 ): Promise<any> {
     const compilerInput = createCompilerInput(
         files,
+        remapping,
         version,
         kind,
         compilationOutput,
@@ -191,10 +189,10 @@ export async function compileSourceString(
     compilerSettings?: any,
     kind?: CompilerKind
 ): Promise<CompileResult> {
-    const entryFileName = normalizeImportPath(fileName);
-    const entryFileDir = path.dirname(entryFileName);
+    const entrySourceUnit = fileName;
+    const entryFileDir = path.dirname(entrySourceUnit);
 
-    const files = new Map([[entryFileName, sourceCode]]);
+    const files = new Map([[entrySourceUnit, sourceCode]]);
     const resolvers = [new FileSystemResolver(), new LocalNpmResolver(entryFileDir)];
 
     resolveFiles(files, remapping, resolvers);
@@ -205,6 +203,7 @@ export async function compileSourceString(
     for (const compilerVersion of compilerVersionStrategy.select()) {
         const data = await compile(
             files,
+            remapping,
             compilerVersion,
             compilationOutput,
             compilerSettings,
@@ -284,6 +283,7 @@ export async function compileJsonData(
         for (const compilerVersion of compilerVersionStrategy.select()) {
             const compileData = await compile(
                 files,
+                [],
                 compilerVersion,
                 compilationOutput,
                 compilerSettings,

test/integration/compile/kinds.spec.ts

@@ -124,7 +124,11 @@ describe(`Native and WASM compilers produce the same results for all files`, ()
     const additionalArgs = new Map<string, [string[], CompilationOutput[], any]>([
         [
             "test/samples/solidity/path_remapping/entry.sol",
-            [["@missing=./local"], defaultCompilationOutput, defaultCompilerSettings]
+            [
+                ["@missing=test/samples/solidity/path_remapping/local"],
+                defaultCompilationOutput,
+                defaultCompilerSettings
+            ]
         ]
     ]);
 

test/unit/compile/inference/findAllFiles.spec.ts

@@ -2,7 +2,7 @@ import expect from "expect";
 import fse from "fs-extra";
 import { join } from "path";
 import { FileSystemResolver } from "../../../../src";
-import { findAllFiles, normalizeImportPath } from "../../../../src/compile/inference";
+import { findAllFiles } from "../../../../src/compile/inference";
 
 const SAMPLES_DIR = join("test", "samples", "solidity");
 
@@ -50,10 +50,9 @@ describe("findAllFiles() find all needed imports", () => {
     for (const [fileName, expectedAllFiles] of samples) {
         it(`All imports for ${fileName} should be ${expectedAllFiles.join(", ")}`, () => {
             const contents = fse.readFileSync(fileName).toString();
-            const files = new Map<string, string>([[normalizeImportPath(fileName), contents]]);
-            const additionalFiles = findAllFiles(files, [], [new FileSystemResolver()]);
-            const allFiles = new Set([normalizeImportPath(fileName), ...additionalFiles.keys()]);
-            expect(allFiles).toEqual(new Set(expectedAllFiles.map(normalizeImportPath)));
+            const files = new Map<string, string>([[fileName, contents]]);
+            findAllFiles(files, [], [new FileSystemResolver()]);
+            expect(new Set(files.keys())).toEqual(new Set(expectedAllFiles));
         });
     }
 });