Treat ambiguous imports as errors

docs/path-resolution.rst

@@ -339,6 +339,15 @@ of the compiler.
 
     Include paths cannot have empty values and must be used together with a non-empty base path.
 
+.. note::
+
+    Include paths and base path can overlap as long as it does not make import resolution ambiguous.
+    For example, you can specify a directory inside base path as an include directory or have an
+    include directory that is a subdirectory of another include directory.
+    The compiler will only issue an error if the source unit name passed to the Host Filesystem
+    Loader represents an existing path when combined with multiple include paths or an include path
+    and base path.
+
 .. _cli-path-normalization-and-stripping:
 
 CLI Path Normalization and Stripping

libsolidity/interface/FileReader.cpp

@@ -21,8 +21,10 @@
 
 #include <libsolutil/CommonIO.h>
 #include <libsolutil/Exceptions.h>
+#include <libsolutil/StringUtils.h>
 
 #include <boost/algorithm/string/predicate.hpp>
+#include <range/v3/view/transform.hpp>
 
 #include <range/v3/range/conversion.hpp>
 
@@ -32,6 +34,7 @@ using solidity::frontend::ReadCallback;
 using solidity::langutil::InternalCompilerError;
 using solidity::util::errinfo_comment;
 using solidity::util::readFileAsString;
+using solidity::util::joinHumanReadable;
 using std::map;
 using std::reference_wrapper;
 using std::string;
@@ -109,24 +112,38 @@ ReadCallback::Result FileReader::readFile(string const& _kind, string const& _so
 		if (strippedSourceUnitName.find("file://") == 0)
 			strippedSourceUnitName.erase(0, 7);
 
+		vector<boost::filesystem::path> candidates;
 		vector<reference_wrapper<boost::filesystem::path>> prefixes = {m_basePath};
 		prefixes += (m_includePaths | ranges::to<vector<reference_wrapper<boost::filesystem::path>>>);
 
-		boost::filesystem::path canonicalPath;
 		for (auto const& prefix: prefixes)
 		{
-			canonicalPath = normalizeCLIPathForVFS(prefix / strippedSourceUnitName, SymlinkResolution::Enabled);
+			boost::filesystem::path canonicalPath = normalizeCLIPathForVFS(prefix / strippedSourceUnitName, SymlinkResolution::Enabled);
 
 			if (boost::filesystem::exists(canonicalPath))
-				break;
+				candidates.push_back(std::move(canonicalPath));
 		}
 
+		auto pathToQuotedString = [](boost::filesystem::path const& _path){ return "\"" + _path.string() + "\""; };
+
+		if (candidates.empty())
+			return ReadCallback::Result{false, "File not found."};
+
+		if (candidates.size() >= 2)
+			return ReadCallback::Result{
+				false,
+				"Ambiguous import. "
+				"Multiple matching files found inside base path and/or include paths: " +
+				joinHumanReadable(candidates | ranges::views::transform(pathToQuotedString), ", ") +
+				"."
+			};
+
 		FileSystemPathSet extraAllowedPaths = {m_basePath.empty() ? "." : m_basePath};
 		extraAllowedPaths += m_includePaths;
 
 		bool isAllowed = false;
 		for (boost::filesystem::path const& allowedDir: m_allowedDirectories + extraAllowedPaths)
-			if (isPathPrefix(normalizeCLIPathForVFS(allowedDir, SymlinkResolution::Enabled), canonicalPath))
+			if (isPathPrefix(normalizeCLIPathForVFS(allowedDir, SymlinkResolution::Enabled), candidates[0]))
 			{
 				isAllowed = true;
 				break;
@@ -135,14 +152,12 @@ ReadCallback::Result FileReader::readFile(string const& _kind, string const& _so
 		if (!isAllowed)
 			return ReadCallback::Result{false, "File outside of allowed directories."};
 
-		if (!boost::filesystem::exists(canonicalPath))
-			return ReadCallback::Result{false, "File not found."};
-
-		if (!boost::filesystem::is_regular_file(canonicalPath))
+		if (!boost::filesystem::is_regular_file(candidates[0]))
 			return ReadCallback::Result{false, "Not a valid file."};
 
 		// NOTE: we ignore the FileNotFound exception as we manually check above
-		auto contents = readFileAsString(canonicalPath);
+		auto contents = readFileAsString(candidates[0]);
+		solAssert(m_sourceCodes.count(_sourceUnitName) == 0, "");
 		m_sourceCodes[_sourceUnitName] = contents;
 		return ReadCallback::Result{true, contents};
 	}

test/solc/CommandLineInterface.cpp

@@ -1247,6 +1247,50 @@ BOOST_AUTO_TEST_CASE(cli_include_paths_should_allow_duplicate_paths)
 	BOOST_TEST(result.reader.basePath() == expectedWorkDir / "dir1/");
 }
 
+BOOST_AUTO_TEST_CASE(cli_include_paths_ambiguous_import)
+{
+	TemporaryDirectory tempDir({"base/", "include/"}, TEST_CASE_NAME);
+	TemporaryWorkingDirectory tempWorkDir(tempDir);
+
+	string const preamble =
+		"// SPDX-License-Identifier: GPL-3.0\n"
+		"pragma solidity >=0.0;\n";
+	string const mainContractSource = preamble +
+		// Ambiguous: both base/contract.sol and include/contract.sol match the import.
+		"import \"contract.sol\";";
+
+	createFilesWithParentDirs({"base/contract.sol", "include/contract.sol"}, preamble);
+
+	boost::filesystem::path expectedWorkDir = "/" / boost::filesystem::canonical(tempDir).relative_path();
+
+	vector<string> commandLine = {
+		"solc",
+		"--no-color",
+		"--base-path=base/",
+		"--include-path=include/",
+		"-",
+	};
+
+	string expectedMessage =
+		"Error: Source \"contract.sol\" not found: Ambiguous import. "
+		"Multiple matching files found inside base path and/or include paths: \"" +
+		(expectedWorkDir / "base/contract.sol").generic_string() + "\", \"" +
+		(expectedWorkDir / "include/contract.sol").generic_string() + "\".\n"
+		" --> <stdin>:3:1:\n"
+		"  |\n"
+		"3 | import \"contract.sol\";\n"
+		"  | ^^^^^^^^^^^^^^^^^^^^^^\n\n";
+
+	OptionsReaderAndMessages result = parseCommandLineAndReadInputFiles(
+		commandLine,
+		mainContractSource,
+		true /* _processInput */
+	);
+
+	BOOST_TEST(result.stderrContent == expectedMessage);
+	BOOST_REQUIRE(!result.success);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 } // namespace solidity::frontend::test

test/solc/CommandLineInterfaceAllowPaths.cpp

@@ -344,10 +344,14 @@ BOOST_FIXTURE_TEST_CASE(allow_path_should_work_with_various_import_forms, AllowP
 	BOOST_TEST(checkImport("import 'a/../../code/a/../a/b/c.sol'", {"--allow-paths", "../code/a/b/c.sol"}));
 	BOOST_TEST(checkImport("import 'a/../../code/a///b/c.sol'", {"--allow-paths", "../code/a/b/c.sol"}));
 
-	// UNC paths in imports
+#if !defined(_WIN32)
+	// UNC paths in imports.
+	// Unfortunately can't test it on Windows without having an existing UNC path. On Linux we can
+	// at least rely on the fact that `//` works like `/`.
 	string uncImportPath = "/" + m_portablePrefix + "/a/b/c.sol";
 	soltestAssert(FileReader::isUNCPath(uncImportPath), "");
 	BOOST_TEST(checkImport("import '" + uncImportPath + "'", {"--allow-paths", "../code/a/b/c.sol"}) == ImportCheck::PathDisallowed());
+#endif
 }
 
 BOOST_FIXTURE_TEST_CASE(allow_path_automatic_whitelisting_input_files, AllowPathsFixture)