From 4f62980d52daa58f21dad3ef7caca5f854395c38 Mon Sep 17 00:00:00 2001 From: VoR0220 Date: Wed, 4 Jan 2017 18:29:54 -0600 Subject: added test Signed-off-by: VoR0220 --- test/libsolidity/Imports.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/libsolidity/Imports.cpp b/test/libsolidity/Imports.cpp index e3f0b281..33978b1f 100644 --- a/test/libsolidity/Imports.cpp +++ b/test/libsolidity/Imports.cpp @@ -172,6 +172,17 @@ BOOST_AUTO_TEST_CASE(filename_with_period) BOOST_CHECK(!c.compile()); } +BOOST_AUTO_TEST_CASE(context_dependent_remappings_ensure_default_and_module_preserved) +{ + CompilerStack c; + c.setRemappings(vector{"foo=vendor/foo_2.0.0", "vendor/bar:foo=vendor/foo_1.0.0", "bar=vendor/bar"}); + c.addSource("main.sol", "import \"foo/foo.sol\"; import {Bar} \"bar/bar.sol\"; contract Main is Foo2, Bar {} pragma solidity >=0.0;"); + c.addSource("vendor/bar/bar.sol", "import \"foo/foo.sol\"; contract Bar is Foo1 {} pragma solidity >=0.0;"); + c.addSource("vendor/foo_1.0.0/foo.sol", "contract Foo1 {} pragma solidity >=0.0;"); + c.addSource("vendor/foo_2.0.0/foo.sol", "contract Foo2 {} pragma solidity >=0.0;"); + BOOST_CHECK(c.compile()); +} + BOOST_AUTO_TEST_SUITE_END() } -- cgit v1.2.3 From 6d9020b3b80aff0baf7d6e023460cfbcd930de6b Mon Sep 17 00:00:00 2001 From: VoR0220 Date: Thu, 5 Jan 2017 12:01:27 -0600 Subject: fixed test and added solution Signed-off-by: VoR0220 --- libsolidity/interface/CompilerStack.cpp | 29 ++++++++++++++++++++++++++--- test/libsolidity/Imports.cpp | 4 ++-- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index ee55f41a..a097e4c8 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -507,21 +507,44 @@ string CompilerStack::applyRemapping(string const& _path, string const& _context return false; return std::equal(_a.begin(), _a.end(), _b.begin()); }; + // Try to find whether _a is a closer match for context _reference than _b + // Defaults to longest prefix in case of a tie. + auto isClosestContext = [](string const& _a, string const& _b, string const& _reference) + { + int a = _reference.compare(_a); + int b = _reference.compare(_b); + if (a == 0) + return true; + else if (b == 0) + return false; + else if (abs(a) == abs(b)) { + return a > 0; + } + return abs(a) < abs(b); + }; + using filepath = boost::filesystem::path; + filepath context(_context); size_t longestPrefix = 0; string longestPrefixTarget; + string currentClosestContext; + string referenceContext = context.parent_path().generic_string(); for (auto const& redir: m_remappings) { + filepath redirContext(redir.context); // Skip if we already have a closer match. - if (longestPrefix > 0 && redir.prefix.length() <= longestPrefix) + if (longestPrefix > 0 && redir.prefix.length() < longestPrefix) continue; // Skip if redir.context is not a prefix of _context - if (!isPrefixOf(redir.context, _context)) + if (!isPrefixOf(redirContext.generic_string(), _context)) continue; // Skip if the prefix does not match. if (!isPrefixOf(redir.prefix, _path)) continue; - + // Skip if there is a prefix collision and the current context is closer + if (redir.prefix.length() == longestPrefix && !isClosestContext(redirContext.generic_string(), currentClosestContext, referenceContext)) + continue; + currentClosestContext = redir.context; longestPrefix = redir.prefix.length(); longestPrefixTarget = redir.target; } diff --git a/test/libsolidity/Imports.cpp b/test/libsolidity/Imports.cpp index 33978b1f..7945f729 100644 --- a/test/libsolidity/Imports.cpp +++ b/test/libsolidity/Imports.cpp @@ -176,8 +176,8 @@ BOOST_AUTO_TEST_CASE(context_dependent_remappings_ensure_default_and_module_pres { CompilerStack c; c.setRemappings(vector{"foo=vendor/foo_2.0.0", "vendor/bar:foo=vendor/foo_1.0.0", "bar=vendor/bar"}); - c.addSource("main.sol", "import \"foo/foo.sol\"; import {Bar} \"bar/bar.sol\"; contract Main is Foo2, Bar {} pragma solidity >=0.0;"); - c.addSource("vendor/bar/bar.sol", "import \"foo/foo.sol\"; contract Bar is Foo1 {} pragma solidity >=0.0;"); + c.addSource("main.sol", "import \"foo/foo.sol\"; import {Bar} from \"bar/bar.sol\"; contract Main is Foo2, Bar {} pragma solidity >=0.0;"); + c.addSource("vendor/bar/bar.sol", "import \"foo/foo.sol\"; contract Bar {Foo1 foo;} pragma solidity >=0.0;"); c.addSource("vendor/foo_1.0.0/foo.sol", "contract Foo1 {} pragma solidity >=0.0;"); c.addSource("vendor/foo_2.0.0/foo.sol", "contract Foo2 {} pragma solidity >=0.0;"); BOOST_CHECK(c.compile()); -- cgit v1.2.3 From 79dbd40096006eac2861dd86dd22fd0c9ab55c6c Mon Sep 17 00:00:00 2001 From: VoR0220 Date: Sat, 7 Jan 2017 12:11:43 -0600 Subject: can do this purely on length. Also made prefix filesystem string for more accurate readings. Signed-off-by: VoR0220 --- libsolidity/interface/CompilerStack.cpp | 43 ++++++++++++--------------------- 1 file changed, 16 insertions(+), 27 deletions(-) diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index a097e4c8..1a95f5a6 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -507,47 +507,36 @@ string CompilerStack::applyRemapping(string const& _path, string const& _context return false; return std::equal(_a.begin(), _a.end(), _b.begin()); }; - // Try to find whether _a is a closer match for context _reference than _b - // Defaults to longest prefix in case of a tie. - auto isClosestContext = [](string const& _a, string const& _b, string const& _reference) - { - int a = _reference.compare(_a); - int b = _reference.compare(_b); - if (a == 0) - return true; - else if (b == 0) - return false; - else if (abs(a) == abs(b)) { - return a > 0; - } - return abs(a) < abs(b); - }; using filepath = boost::filesystem::path; - filepath context(_context); size_t longestPrefix = 0; + size_t longestContext = 0; string longestPrefixTarget; - string currentClosestContext; - string referenceContext = context.parent_path().generic_string(); + for (auto const& redir: m_remappings) { filepath redirContext(redir.context); - // Skip if we already have a closer match. - if (longestPrefix > 0 && redir.prefix.length() < longestPrefix) + filepath redirPrefix(redir.prefix); + string contextFileString = redirContext.generic_string(); + string prefixFileString = redirPrefix.generic_string(); + // Skip if there is a prefix collision and the current context is closer + if (longestContext > 0 && contextFileString.length() < longestContext) continue; // Skip if redir.context is not a prefix of _context - if (!isPrefixOf(redirContext.generic_string(), _context)) + if (!isPrefixOf(contextFileString, _context)) continue; - // Skip if the prefix does not match. - if (!isPrefixOf(redir.prefix, _path)) + // Skip if we already have a closer match. + if (longestPrefix > 0 && prefixFileString.length() < longestPrefix) continue; - // Skip if there is a prefix collision and the current context is closer - if (redir.prefix.length() == longestPrefix && !isClosestContext(redirContext.generic_string(), currentClosestContext, referenceContext)) + // Skip if the prefix does not match. + if (!isPrefixOf(prefixFileString, _path)) continue; - currentClosestContext = redir.context; - longestPrefix = redir.prefix.length(); + + longestContext = contextFileString.length(); + longestPrefix = prefixFileString.length(); longestPrefixTarget = redir.target; } + string path = longestPrefixTarget; path.append(_path.begin() + longestPrefix, _path.end()); return path; -- cgit v1.2.3 From 8ace851831f63035174ae086bb415a0d2efb8277 Mon Sep 17 00:00:00 2001 From: VoR0220 Date: Tue, 10 Jan 2017 07:15:48 -0600 Subject: much smaller helper function Signed-off-by: VoR0220 --- libsolidity/interface/CompilerStack.cpp | 6 ++---- libsolidity/interface/CompilerStack.h | 4 +++- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index 1a95f5a6..094360a3 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -515,10 +515,8 @@ string CompilerStack::applyRemapping(string const& _path, string const& _context for (auto const& redir: m_remappings) { - filepath redirContext(redir.context); - filepath redirPrefix(redir.prefix); - string contextFileString = redirContext.generic_string(); - string prefixFileString = redirPrefix.generic_string(); + string contextFileString = sanitizePath(redir.context); + string prefixFileString = sanitizePath(redir.prefix); // Skip if there is a prefix collision and the current context is closer if (longestContext > 0 && contextFileString.length() < longestContext) continue; diff --git a/libsolidity/interface/CompilerStack.h b/libsolidity/interface/CompilerStack.h index f98a457a..ef0aa6fe 100644 --- a/libsolidity/interface/CompilerStack.h +++ b/libsolidity/interface/CompilerStack.h @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -239,7 +240,8 @@ private: ContractDefinition const& _contract, std::map& _compiledContracts ); - + /// Helper function to return path converted strings. + std::string sanitizePath(std::string const& _path) { return boost::filesystem::path(_path).generic_string(); } void link(); Contract const& contract(std::string const& _contractName = "") const; -- cgit v1.2.3 From e02270bbb4a9f151bc25e0e0f96f1e4505542de5 Mon Sep 17 00:00:00 2001 From: VoR0220 Date: Wed, 11 Jan 2017 09:03:41 -0600 Subject: fixed unused filepath bug Signed-off-by: VoR0220 --- libsolidity/interface/CompilerStack.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index 094360a3..30abad07 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -508,7 +508,6 @@ string CompilerStack::applyRemapping(string const& _path, string const& _context return std::equal(_a.begin(), _a.end(), _b.begin()); }; - using filepath = boost::filesystem::path; size_t longestPrefix = 0; size_t longestContext = 0; string longestPrefixTarget; @@ -524,7 +523,7 @@ string CompilerStack::applyRemapping(string const& _path, string const& _context if (!isPrefixOf(contextFileString, _context)) continue; // Skip if we already have a closer match. - if (longestPrefix > 0 && prefixFileString.length() < longestPrefix) + if (prefixFileString.length() < longestPrefix) continue; // Skip if the prefix does not match. if (!isPrefixOf(prefixFileString, _path)) -- cgit v1.2.3 From e96c32a072160428a93720a21c51cd8670d5e664 Mon Sep 17 00:00:00 2001 From: VoR0220 Date: Wed, 11 Jan 2017 09:32:07 -0600 Subject: changelog entry Signed-off-by: VoR0220 --- Changelog.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Changelog.md b/Changelog.md index 0ac0cb2f..c06313ba 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,6 +1,8 @@ ### 0.4.8 (unreleased) BugFixes: + * Remappings: a=b would overwrite c:a=d. This has now been fixed to all modules except + c using b as their target, with c using d as the target. * Type checker, code generator: enable access to events of base contracts' names. * Imports: ``import ".dir/a"`` is not a relative path. Relative paths begin with directory ``.`` or ``..``. -- cgit v1.2.3 From 4585bfdce7716cd4837f71b565bb9a6dff8c2d7e Mon Sep 17 00:00:00 2001 From: VoR0220 Date: Wed, 11 Jan 2017 10:56:35 -0600 Subject: cleanup Signed-off-by: VoR0220 fixup Signed-off-by: VoR0220 --- Changelog.md | 3 +-- libsolidity/interface/CompilerStack.cpp | 24 ++++++++++++------------ libsolidity/interface/CompilerStack.h | 5 +++-- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/Changelog.md b/Changelog.md index c06313ba..52f86d40 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,8 +1,7 @@ ### 0.4.8 (unreleased) BugFixes: - * Remappings: a=b would overwrite c:a=d. This has now been fixed to all modules except - c using b as their target, with c using d as the target. + * Remappings: Prefer longer context over longer prefix. * Type checker, code generator: enable access to events of base contracts' names. * Imports: ``import ".dir/a"`` is not a relative path. Relative paths begin with directory ``.`` or ``..``. diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index 30abad07..1e571cae 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -510,28 +510,28 @@ string CompilerStack::applyRemapping(string const& _path, string const& _context size_t longestPrefix = 0; size_t longestContext = 0; - string longestPrefixTarget; + string bestMatchTarget; for (auto const& redir: m_remappings) { - string contextFileString = sanitizePath(redir.context); - string prefixFileString = sanitizePath(redir.prefix); - // Skip if there is a prefix collision and the current context is closer - if (longestContext > 0 && contextFileString.length() < longestContext) + string context = sanitizePath(redir.context); + string prefix = sanitizePath(redir.prefix); + // Skip if current context is closer + if (context.length() < longestContext) continue; // Skip if redir.context is not a prefix of _context - if (!isPrefixOf(contextFileString, _context)) + if (!isPrefixOf(context, _context)) continue; - // Skip if we already have a closer match. - if (prefixFileString.length() < longestPrefix) + // Skip if we already have a closer prefix match. + if (prefix.length() < longestPrefix) continue; // Skip if the prefix does not match. - if (!isPrefixOf(prefixFileString, _path)) + if (!isPrefixOf(prefix, _path)) continue; - longestContext = contextFileString.length(); - longestPrefix = prefixFileString.length(); - longestPrefixTarget = redir.target; + longestContext = context.length(); + longestPrefix = prefix.length(); + bestMatchTarget = redir.target; } string path = longestPrefixTarget; diff --git a/libsolidity/interface/CompilerStack.h b/libsolidity/interface/CompilerStack.h index ef0aa6fe..d49a8df1 100644 --- a/libsolidity/interface/CompilerStack.h +++ b/libsolidity/interface/CompilerStack.h @@ -235,13 +235,14 @@ private: bool checkLibraryNameClashes(); /// @returns the absolute path corresponding to @a _path relative to @a _reference. std::string absolutePath(std::string const& _path, std::string const& _reference) const; + /// Helper function to return path converted strings. + std::string sanitizePath(std::string const& _path) const { return boost::filesystem::path(_path).generic_string(); } + /// Compile a single contract and put the result in @a _compiledContracts. void compileContract( ContractDefinition const& _contract, std::map& _compiledContracts ); - /// Helper function to return path converted strings. - std::string sanitizePath(std::string const& _path) { return boost::filesystem::path(_path).generic_string(); } void link(); Contract const& contract(std::string const& _contractName = "") const; -- cgit v1.2.3 From 4542f459f165502ed9537bb570de44640cdc4228 Mon Sep 17 00:00:00 2001 From: VoR0220 Date: Wed, 11 Jan 2017 11:45:14 -0600 Subject: added fix and a test for order independence of nested prefixing Signed-off-by: VoR0220 --- libsolidity/interface/CompilerStack.cpp | 4 ++-- test/libsolidity/Imports.cpp | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index 1e571cae..e4c351ff 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -523,7 +523,7 @@ string CompilerStack::applyRemapping(string const& _path, string const& _context if (!isPrefixOf(context, _context)) continue; // Skip if we already have a closer prefix match. - if (prefix.length() < longestPrefix) + if (prefix.length() < longestPrefix && context.length() == longestContext) continue; // Skip if the prefix does not match. if (!isPrefixOf(prefix, _path)) @@ -534,7 +534,7 @@ string CompilerStack::applyRemapping(string const& _path, string const& _context bestMatchTarget = redir.target; } - string path = longestPrefixTarget; + string path = bestMatchTarget; path.append(_path.begin() + longestPrefix, _path.end()); return path; } diff --git a/test/libsolidity/Imports.cpp b/test/libsolidity/Imports.cpp index 7945f729..712f133a 100644 --- a/test/libsolidity/Imports.cpp +++ b/test/libsolidity/Imports.cpp @@ -183,6 +183,24 @@ BOOST_AUTO_TEST_CASE(context_dependent_remappings_ensure_default_and_module_pres BOOST_CHECK(c.compile()); } +BOOST_AUTO_TEST_CASE(context_dependent_remappings_order_independent) +{ + CompilerStack c; + c.setRemappings(vector{"a:x/y/z=d", "a/b:x=e"}); + c.addSource("a/main.sol", "import \"x/y/z/z.sol\"; contract Main is D {} pragma solidity >=0.0;"); + c.addSource("a/b/main.sol", "import \"x/y/z/z.sol\"; contract Main is E {} pragma solidity >=0.0;"); + c.addSource("x/y/z/z.sol", "contract D {} pragma solidity >=0.0;"); + c.addSource("e/y/z/z.sol", "contract E {} pragma solidity >=0.0;"); + BOOST_CHECK(c.compile()); + CompilerStack d; + d.setRemappings(vector{"a/b:x=e", "a:x/y/z=d"}); + d.addSource("a/main.sol", "import \"x/y/z/z.sol\"; contract Main is D {} pragma solidity >=0.0;"); + d.addSource("a/b/main.sol", "import \"x/y/z/z.sol\"; contract Main is E {} pragma solidity >=0.0;"); + d.addSource("x/y/z/z.sol", "contract D {} pragma solidity >=0.0;"); + d.addSource("e/y/z/z.sol", "contract E {} pragma solidity >=0.0;"); + BOOST_CHECK(d.compile()); +} + BOOST_AUTO_TEST_SUITE_END() } -- cgit v1.2.3 From b6508ca992531154a572320bf8bb117e3b9294b9 Mon Sep 17 00:00:00 2001 From: VoR0220 Date: Wed, 11 Jan 2017 12:03:54 -0600 Subject: fixed Signed-off-by: VoR0220 --- libsolidity/interface/CompilerStack.cpp | 4 ++-- test/libsolidity/Imports.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index e4c351ff..a31df584 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -516,6 +516,7 @@ string CompilerStack::applyRemapping(string const& _path, string const& _context { string context = sanitizePath(redir.context); string prefix = sanitizePath(redir.prefix); + // Skip if current context is closer if (context.length() < longestContext) continue; @@ -531,9 +532,8 @@ string CompilerStack::applyRemapping(string const& _path, string const& _context longestContext = context.length(); longestPrefix = prefix.length(); - bestMatchTarget = redir.target; + bestMatchTarget = sanitizePath(redir.target); } - string path = bestMatchTarget; path.append(_path.begin() + longestPrefix, _path.end()); return path; diff --git a/test/libsolidity/Imports.cpp b/test/libsolidity/Imports.cpp index 712f133a..56895fdc 100644 --- a/test/libsolidity/Imports.cpp +++ b/test/libsolidity/Imports.cpp @@ -189,14 +189,14 @@ BOOST_AUTO_TEST_CASE(context_dependent_remappings_order_independent) c.setRemappings(vector{"a:x/y/z=d", "a/b:x=e"}); c.addSource("a/main.sol", "import \"x/y/z/z.sol\"; contract Main is D {} pragma solidity >=0.0;"); c.addSource("a/b/main.sol", "import \"x/y/z/z.sol\"; contract Main is E {} pragma solidity >=0.0;"); - c.addSource("x/y/z/z.sol", "contract D {} pragma solidity >=0.0;"); + c.addSource("d/z.sol", "contract D {} pragma solidity >=0.0;"); c.addSource("e/y/z/z.sol", "contract E {} pragma solidity >=0.0;"); BOOST_CHECK(c.compile()); CompilerStack d; d.setRemappings(vector{"a/b:x=e", "a:x/y/z=d"}); d.addSource("a/main.sol", "import \"x/y/z/z.sol\"; contract Main is D {} pragma solidity >=0.0;"); d.addSource("a/b/main.sol", "import \"x/y/z/z.sol\"; contract Main is E {} pragma solidity >=0.0;"); - d.addSource("x/y/z/z.sol", "contract D {} pragma solidity >=0.0;"); + d.addSource("d/z.sol", "contract D {} pragma solidity >=0.0;"); d.addSource("e/y/z/z.sol", "contract E {} pragma solidity >=0.0;"); BOOST_CHECK(d.compile()); } -- cgit v1.2.3