diff --git a/Changelog.md b/Changelog.md index 1e40a0b73..f2954305c 100644 --- a/Changelog.md +++ b/Changelog.md @@ -16,6 +16,7 @@ Bugfixes: * Fix internal error when popping a dynamic storage array of mappings. * Yul Optimizer: Fix reordering bug in connection with shifted one and mul/div-instructions in for loop conditions. * Scanner: Fix multi-line natspec comment parsing with triple slashes when file is encoded with CRLF instead of LF. + * Name Resolver: Fix wrong source location when warning on shadowed aliases in import declarations. ### 0.5.11 (2019-08-12) diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp index 727dc5f35..e9792ddf2 100644 --- a/libsolidity/analysis/NameAndTypeResolver.cpp +++ b/libsolidity/analysis/NameAndTypeResolver.cpp @@ -91,13 +91,13 @@ bool NameAndTypeResolver::performImports(SourceUnit& _sourceUnit, mapsymbolAliases().empty()) for (auto const& alias: imp->symbolAliases()) { - auto declarations = scope->second->resolveName(alias.first->name(), false); + auto declarations = scope->second->resolveName(alias.symbol->name(), false); if (declarations.empty()) { m_errorReporter.declarationError( imp->location(), "Declaration \"" + - alias.first->name() + + alias.symbol->name() + "\" not found in \"" + path + "\" (referenced as \"" + @@ -109,7 +109,7 @@ bool NameAndTypeResolver::performImports(SourceUnit& _sourceUnit, maplocation(), true, false, m_errorReporter + target, *declaration, alias.alias.get(), &alias.location, true, false, m_errorReporter )) error = true; } @@ -523,7 +523,7 @@ bool DeclarationRegistrationHelper::registerDeclaration( { if (dynamic_cast(shadowedDeclaration)) _errorReporter.warning( - _declaration.location(), + *_errorLocation, "This declaration shadows a builtin symbol." ); else diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h index b84ad2c19..4d03bf4ec 100644 --- a/libsolidity/ast/AST.h +++ b/libsolidity/ast/AST.h @@ -280,22 +280,31 @@ private: class ImportDirective: public Declaration { public: + struct SymbolAlias + { + ASTPointer symbol; + ASTPointer alias; + SourceLocation location; + }; + + using SymbolAliasList = std::vector; + ImportDirective( SourceLocation const& _location, ASTPointer const& _path, ASTPointer const& _unitAlias, - std::vector, ASTPointer>>&& _symbolAliases + SymbolAliasList _symbolAliases ): Declaration(_location, _unitAlias), m_path(_path), - m_symbolAliases(_symbolAliases) + m_symbolAliases(move(_symbolAliases)) { } void accept(ASTVisitor& _visitor) override; void accept(ASTConstVisitor& _visitor) const override; ASTString const& path() const { return *m_path; } - std::vector, ASTPointer>> const& symbolAliases() const + SymbolAliasList const& symbolAliases() const { return m_symbolAliases; } @@ -306,9 +315,9 @@ public: private: ASTPointer m_path; /// The aliases for the specific symbols to import. If non-empty import the specific symbols. - /// If the second component is empty, import the identifier unchanged. + /// If the `alias` component is empty, import the identifier unchanged. /// If both m_unitAlias and m_symbolAlias are empty, import all symbols into the current scope. - std::vector, ASTPointer>> m_symbolAliases; + SymbolAliasList m_symbolAliases; }; /** diff --git a/libsolidity/ast/ASTJsonConverter.cpp b/libsolidity/ast/ASTJsonConverter.cpp index 669b46d53..5424ee5bc 100644 --- a/libsolidity/ast/ASTJsonConverter.cpp +++ b/libsolidity/ast/ASTJsonConverter.cpp @@ -243,9 +243,9 @@ bool ASTJsonConverter::visit(ImportDirective const& _node) for (auto const& symbolAlias: _node.symbolAliases()) { Json::Value tuple(Json::objectValue); - solAssert(symbolAlias.first, ""); - tuple["foreign"] = nodeId(*symbolAlias.first); - tuple["local"] = symbolAlias.second ? Json::Value(*symbolAlias.second) : Json::nullValue; + solAssert(symbolAlias.symbol, ""); + tuple["foreign"] = nodeId(*symbolAlias.symbol); + tuple["local"] = symbolAlias.alias ? Json::Value(*symbolAlias.alias) : Json::nullValue; symbolAliases.append(tuple); } attributes.emplace_back("symbolAliases", std::move(symbolAliases)); diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp index f91cb91ea..56a74f47d 100644 --- a/libsolidity/parsing/Parser.cpp +++ b/libsolidity/parsing/Parser.cpp @@ -180,7 +180,7 @@ ASTPointer Parser::parseImportDirective() expectToken(Token::Import); ASTPointer path; ASTPointer unitAlias = make_shared(); - vector, ASTPointer>> symbolAliases; + ImportDirective::SymbolAliasList symbolAliases; if (m_scanner->currentToken() == Token::StringLiteral) { @@ -198,14 +198,16 @@ ASTPointer Parser::parseImportDirective() m_scanner->next(); while (true) { - ASTPointer id = parseIdentifier(); ASTPointer alias; + SourceLocation aliasLocation = SourceLocation{position(), endPosition(), source()}; + ASTPointer id = parseIdentifier(); if (m_scanner->currentToken() == Token::As) { expectToken(Token::As); + aliasLocation = SourceLocation{position(), endPosition(), source()}; alias = expectIdentifierToken(); } - symbolAliases.emplace_back(move(id), move(alias)); + symbolAliases.emplace_back(ImportDirective::SymbolAlias{move(id), move(alias), aliasLocation}); if (m_scanner->currentToken() != Token::Comma) break; m_scanner->next(); diff --git a/test/libsolidity/syntaxTests/imports/library_name_clash.sol b/test/libsolidity/syntaxTests/imports/library_name_clash.sol index cd0dbd240..19f3697d5 100644 --- a/test/libsolidity/syntaxTests/imports/library_name_clash.sol +++ b/test/libsolidity/syntaxTests/imports/library_name_clash.sol @@ -5,4 +5,4 @@ library A {} ==== Source: c ==== import {A} from "./a"; import {A} from "./b"; // ---- -// DeclarationError: (c:23-45): Identifier already declared. +// DeclarationError: (c:31-32): Identifier already declared. diff --git a/test/libsolidity/syntaxTests/imports/shadowing_builtins_with_alias.sol b/test/libsolidity/syntaxTests/imports/shadowing_builtins_with_alias.sol index 8ed652a71..65a606e0c 100644 --- a/test/libsolidity/syntaxTests/imports/shadowing_builtins_with_alias.sol +++ b/test/libsolidity/syntaxTests/imports/shadowing_builtins_with_alias.sol @@ -3,4 +3,4 @@ contract C {} ==== Source: b ==== import {C as msg} from "B.sol"; // ---- -// Warning: (B.sol:0-13): This declaration shadows a builtin symbol. +// Warning: (b:13-16): This declaration shadows a builtin symbol. diff --git a/test/libsolidity/syntaxTests/imports/shadowing_builtins_with_multiple_imports.sol b/test/libsolidity/syntaxTests/imports/shadowing_builtins_with_multiple_imports.sol index 848e15546..3d82041af 100644 --- a/test/libsolidity/syntaxTests/imports/shadowing_builtins_with_multiple_imports.sol +++ b/test/libsolidity/syntaxTests/imports/shadowing_builtins_with_multiple_imports.sol @@ -7,5 +7,5 @@ contract C { // ---- // Warning: (B.sol:0-15): This declaration shadows a builtin symbol. // Warning: (B.sol:16-32): This declaration shadows a builtin symbol. -// Warning: (B.sol:0-15): This declaration shadows a builtin symbol. -// Warning: (B.sol:16-32): This declaration shadows a builtin symbol. +// Warning: (b:8-11): This declaration shadows a builtin symbol. +// Warning: (b:13-18): This declaration shadows a builtin symbol. diff --git a/test/libsolidity/syntaxTests/imports/shadowing_via_import.sol b/test/libsolidity/syntaxTests/imports/shadowing_via_import.sol index cd0dbd240..19f3697d5 100644 --- a/test/libsolidity/syntaxTests/imports/shadowing_via_import.sol +++ b/test/libsolidity/syntaxTests/imports/shadowing_via_import.sol @@ -5,4 +5,4 @@ library A {} ==== Source: c ==== import {A} from "./a"; import {A} from "./b"; // ---- -// DeclarationError: (c:23-45): Identifier already declared. +// DeclarationError: (c:31-32): Identifier already declared.