Detect source unit name collisions between paths specified on the command line

This commit is contained in:
Kamil Śliwak 2021-08-26 18:34:34 +02:00
parent c8a7a1da7c
commit 9975b5e26b
5 changed files with 143 additions and 0 deletions

View File

@ -392,6 +392,16 @@ Otherwise the file path remains absolute.
This makes the conversion unambiguous and ensures that the relative path does not start with ``../``. This makes the conversion unambiguous and ensures that the relative path does not start with ``../``.
The resulting file path becomes the source unit name. The resulting file path becomes the source unit name.
.. note::
The relative path produced by stripping must remain unique within the base path and include paths.
For example the compiler will issue an error for the following command if both
``/project/contract.sol`` and ``/lib/contract.sol`` exist:
.. code-block:: bash
solc /project/contract.sol --base-path /project --include-path /lib
.. note:: .. note::
Prior to version 0.8.8, CLI path stripping was not performed and the only normalization applied Prior to version 0.8.8, CLI path stripping was not performed and the only normalization applied

View File

@ -32,6 +32,7 @@ using solidity::frontend::ReadCallback;
using solidity::langutil::InternalCompilerError; using solidity::langutil::InternalCompilerError;
using solidity::util::errinfo_comment; using solidity::util::errinfo_comment;
using solidity::util::readFileAsString; using solidity::util::readFileAsString;
using std::map;
using std::reference_wrapper; using std::reference_wrapper;
using std::string; using std::string;
using std::vector; using std::vector;
@ -176,6 +177,24 @@ string FileReader::cliPathToSourceUnitName(boost::filesystem::path const& _cliPa
return normalizedPath.generic_string(); return normalizedPath.generic_string();
} }
map<string, FileReader::FileSystemPathSet> FileReader::detectSourceUnitNameCollisions(FileSystemPathSet const& _cliPaths)
{
map<string, FileReader::FileSystemPathSet> nameToPaths;
for (boost::filesystem::path const& cliPath: _cliPaths)
{
string sourceUnitName = cliPathToSourceUnitName(cliPath);
boost::filesystem::path normalizedPath = normalizeCLIPathForVFS(cliPath);
nameToPaths[sourceUnitName].insert(normalizedPath);
}
map<string, FileReader::FileSystemPathSet> collisions;
for (auto&& [sourceUnitName, cliPaths]: nameToPaths)
if (cliPaths.size() >= 2)
collisions[sourceUnitName] = std::move(cliPaths);
return collisions;
}
boost::filesystem::path FileReader::normalizeCLIPathForVFS( boost::filesystem::path FileReader::normalizeCLIPathForVFS(
boost::filesystem::path const& _path, boost::filesystem::path const& _path,
SymlinkResolution _symlinkResolution SymlinkResolution _symlinkResolution

View File

@ -96,6 +96,13 @@ public:
/// making it relative to base path or one of the include directories. /// making it relative to base path or one of the include directories.
std::string cliPathToSourceUnitName(boost::filesystem::path const& _cliPath); std::string cliPathToSourceUnitName(boost::filesystem::path const& _cliPath);
/// Checks if a set contains any paths that lead to different files but would receive identical
/// source unit names. Files are considered the same if their paths are exactly the same after
/// normalization (without following symlinks).
/// @returns a map containing all the conflicting source unit names and the paths that would
/// receive them. The returned paths are normalized.
std::map<std::string, FileSystemPathSet> detectSourceUnitNameCollisions(FileSystemPathSet const& _cliPaths);
/// Normalizes a filesystem path to make it include all components up to the filesystem root, /// Normalizes a filesystem path to make it include all components up to the filesystem root,
/// remove small, inconsequential differences that do not affect the meaning and make it look /// remove small, inconsequential differences that do not affect the meaning and make it look
/// the same on all platforms (if possible). /// the same on all platforms (if possible).

View File

@ -428,6 +428,24 @@ bool CommandLineInterface::readInputFiles()
for (boost::filesystem::path const& allowedDirectory: m_options.input.allowedDirectories) for (boost::filesystem::path const& allowedDirectory: m_options.input.allowedDirectories)
m_fileReader.allowDirectory(allowedDirectory); m_fileReader.allowDirectory(allowedDirectory);
map<std::string, set<boost::filesystem::path>> collisions =
m_fileReader.detectSourceUnitNameCollisions(m_options.input.paths);
if (!collisions.empty())
{
auto pathToQuotedString = [](boost::filesystem::path const& _path){ return "\"" + _path.string() + "\""; };
serr() << "Source unit name collision detected. ";
serr() << "The specified values of base path and/or include paths would result in multiple ";
serr() << "input files being assigned the same source unit name:" << endl;
for (auto const& [sourceUnitName, normalizedInputPaths]: collisions)
{
serr() << sourceUnitName << " matches: ";
serr() << joinHumanReadable(normalizedInputPaths | ranges::views::transform(pathToQuotedString)) << endl;
}
return false;
}
for (boost::filesystem::path const& infile: m_options.input.paths) for (boost::filesystem::path const& infile: m_options.input.paths)
{ {
if (!boost::filesystem::exists(infile)) if (!boost::filesystem::exists(infile))

View File

@ -1112,6 +1112,95 @@ BOOST_AUTO_TEST_CASE(cli_include_paths_without_base_path)
BOOST_TEST(result.stderrContent == expectedMessage); BOOST_TEST(result.stderrContent == expectedMessage);
} }
BOOST_AUTO_TEST_CASE(cli_include_paths_should_detect_source_unit_name_collisions)
{
TemporaryDirectory tempDir({"dir1/", "dir2/", "dir3/"}, TEST_CASE_NAME);
TemporaryWorkingDirectory tempWorkDir(tempDir);
createFilesWithParentDirs({
"dir1/contract1.sol",
"dir1/contract2.sol",
"dir2/contract1.sol",
"dir2/contract2.sol",
});
boost::filesystem::path expectedWorkDir = "/" / boost::filesystem::canonical(tempDir).relative_path();
string expectedMessage =
"Source unit name collision detected. "
"The specified values of base path and/or include paths would result in multiple "
"input files being assigned the same source unit name:\n"
"contract1.sol matches: "
"\"" + (expectedWorkDir / "dir1/contract1.sol").generic_string() + "\", "
"\"" + (expectedWorkDir / "dir2/contract1.sol").generic_string() + "\"\n"
"contract2.sol matches: "
"\"" + (expectedWorkDir / "dir1/contract2.sol").generic_string() + "\", "
"\"" + (expectedWorkDir / "dir2/contract2.sol").generic_string() + "\"\n";
{
// import "contract1.sol" and import "contract2.sol" would be ambiguous:
vector<string> commandLine = {
"solc",
"--base-path=dir1/",
"--include-path=dir2/",
"dir1/contract1.sol",
"dir2/contract1.sol",
"dir1/contract2.sol",
"dir2/contract2.sol",
};
OptionsReaderAndMessages result = parseCommandLineAndReadInputFiles(commandLine);
BOOST_TEST(result.stderrContent == expectedMessage);
BOOST_REQUIRE(!result.success);
}
{
// import "contract1.sol" and import "contract2.sol" would be ambiguous:
vector<string> commandLine = {
"solc",
"--base-path=dir3/",
"--include-path=dir1/",
"--include-path=dir2/",
"dir1/contract1.sol",
"dir2/contract1.sol",
"dir1/contract2.sol",
"dir2/contract2.sol",
};
OptionsReaderAndMessages result = parseCommandLineAndReadInputFiles(commandLine);
BOOST_TEST(result.stderrContent == expectedMessage);
BOOST_REQUIRE(!result.success);
}
{
// No conflict if files with the same name exist but only one is given to the compiler.
vector<string> commandLine = {
"solc",
"--base-path=dir3/",
"--include-path=dir1/",
"--include-path=dir2/",
"dir1/contract1.sol",
"dir1/contract2.sol",
};
OptionsReaderAndMessages result = parseCommandLineAndReadInputFiles(commandLine);
BOOST_TEST(result.stderrContent == "");
BOOST_REQUIRE(result.success);
}
{
// The same file specified multiple times is not a conflict.
vector<string> commandLine = {
"solc",
"--base-path=dir3/",
"--include-path=dir1/",
"--include-path=dir2/",
"dir1/contract1.sol",
"dir1/contract1.sol",
"./dir1/contract1.sol",
};
OptionsReaderAndMessages result = parseCommandLineAndReadInputFiles(commandLine);
BOOST_TEST(result.stderrContent == "");
BOOST_REQUIRE(result.success);
}
}
BOOST_AUTO_TEST_CASE(cli_include_paths_should_allow_duplicate_paths) BOOST_AUTO_TEST_CASE(cli_include_paths_should_allow_duplicate_paths)
{ {
TemporaryDirectory tempDir({"dir1/", "dir2/"}, TEST_CASE_NAME); TemporaryDirectory tempDir({"dir1/", "dir2/"}, TEST_CASE_NAME);