Contract level checker: Disallow free function redefinition and alias

shadowing another free function

Co-authored-by: chriseth <chris@ethereum.org>

libsolidity/analysis/ContractLevelChecker.cpp

@@ -38,15 +38,45 @@ namespace
 {
 
 template <class T, class B>
-bool hasEqualNameAndParameters(T const& _a, B const& _b)
+bool hasEqualParameters(T const& _a, B const& _b)
 {
-	return
-		_a.name() == _b.name() &&
-		FunctionType(_a).asExternallyCallableFunction(false)->hasEqualParameterTypes(
-			*FunctionType(_b).asExternallyCallableFunction(false)
-		);
+	return FunctionType(_a).asExternallyCallableFunction(false)->hasEqualParameterTypes(
+		*FunctionType(_b).asExternallyCallableFunction(false)
+	);
 }
 
+template<typename T>
+map<ASTString, vector<T const*>> filterDeclarations(
+	map<ASTString, vector<Declaration const*>> const& _declarations)
+{
+	map<ASTString, vector<T const*>> filteredDeclarations;
+	for (auto const& [name, overloads]: _declarations)
+		for (auto const* declaration: overloads)
+			if (auto typedDeclaration = dynamic_cast<T const*>(declaration))
+				filteredDeclarations[name].push_back(typedDeclaration);
+	return filteredDeclarations;
+}
+
+}
+
+bool ContractLevelChecker::check(SourceUnit const& _sourceUnit)
+{
+	bool noErrors = true;
+	findDuplicateDefinitions(
+		filterDeclarations<FunctionDefinition>(*_sourceUnit.annotation().exportedSymbols)
+	);
+	// This check flags duplicate free events when free events become
+	// a Solidity feature
+	findDuplicateDefinitions(
+		filterDeclarations<EventDefinition>(*_sourceUnit.annotation().exportedSymbols)
+	);
+	if (!Error::containsOnlyWarnings(m_errorReporter.errors()))
+		noErrors = false;
+	for (ASTPointer<ASTNode> const& node: _sourceUnit.nodes())
+		if (ContractDefinition* contract = dynamic_cast<ContractDefinition*>(node.get()))
+			if (!check(*contract))
+				noErrors = false;
+	return noErrors;
 }
 
 bool ContractLevelChecker::check(ContractDefinition const& _contract)
@@ -143,8 +173,21 @@ void ContractLevelChecker::findDuplicateDefinitions(map<string, vector<T>> const
 			SecondarySourceLocation ssl;
 
 			for (size_t j = i + 1; j < overloads.size(); ++j)
-				if (hasEqualNameAndParameters(*overloads[i], *overloads[j]))
+				if (hasEqualParameters(*overloads[i], *overloads[j]))
 				{
+					solAssert(
+						(
+							dynamic_cast<ContractDefinition const*>(overloads[i]->scope()) &&
+							dynamic_cast<ContractDefinition const*>(overloads[j]->scope()) &&
+							overloads[i]->name() == overloads[j]->name()
+						) ||
+						(
+							dynamic_cast<SourceUnit const*>(overloads[i]->scope()) &&
+							dynamic_cast<SourceUnit const*>(overloads[j]->scope())
+						),
+						"Override is neither a namesake function/event in contract scope nor "
+						"a free function/event (alias)."
+					);
 					ssl.append("Other declaration is here:", overloads[j]->location());
 					reported.insert(j);
 				}

libsolidity/analysis/ContractLevelChecker.h

@@ -39,7 +39,7 @@ namespace solidity::frontend
 
 /**
  * Component that verifies overloads, abstract contracts, function clashes and others
- * checks at contract or function level.
+ * checks at file, contract, or function level.
  */
 class ContractLevelChecker
 {
@@ -51,11 +51,14 @@ public:
 		m_errorReporter(_errorReporter)
 	{}
 
+	/// Performs checks on the given source ast.
+	/// @returns true iff all checks passed. Note even if all checks passed, errors() can still contain warnings
+	bool check(SourceUnit const& _sourceUnit);
+
+private:
 	/// Performs checks on the given contract.
 	/// @returns true iff all checks passed. Note even if all checks passed, errors() can still contain warnings
 	bool check(ContractDefinition const& _contract);
-
-private:
 	/// Checks that two functions defined in this contract with the same name have different
 	/// arguments and that there is at most one constructor.
 	void checkDuplicateFunctions(ContractDefinition const& _contract);

libsolidity/analysis/NameAndTypeResolver.cpp

@@ -121,6 +121,7 @@ bool NameAndTypeResolver::performImports(SourceUnit& _sourceUnit, map<string, So
 						))
 							error =  true;
 		}
+	_sourceUnit.annotation().exportedSymbols = m_scopes[&_sourceUnit]->declarations();
 	return !error;
 }
 
@@ -533,7 +534,6 @@ bool DeclarationRegistrationHelper::visit(SourceUnit& _sourceUnit)
 
 void DeclarationRegistrationHelper::endVisit(SourceUnit& _sourceUnit)
 {
-	_sourceUnit.annotation().exportedSymbols = m_scopes[&_sourceUnit]->declarations();
 	ASTVisitor::endVisit(_sourceUnit);
 }
 

libsolidity/interface/CompilerStack.cpp

@@ -366,12 +366,10 @@ bool CompilerStack::analyze()
 		// This also calculates whether a contract is abstract, which is needed by the
 		// type checker.
 		ContractLevelChecker contractLevelChecker(m_errorReporter);
+
 		for (Source const* source: m_sourceOrder)
-			if (source->ast)
-				for (ASTPointer<ASTNode> const& node: source->ast->nodes())
-					if (ContractDefinition* contract = dynamic_cast<ContractDefinition*>(node.get()))
-						if (!contractLevelChecker.check(*contract))
-							noErrors = false;
+			if (auto sourceAst = source->ast)
+				noErrors = contractLevelChecker.check(*sourceAst);
 
 		// Requires ContractLevelChecker
 		DocStringAnalyser docStringAnalyser(m_errorReporter);