Merge pull request #2605 from roadriverrail/this-in-constructor

This in constructor

Changelog.md

@@ -7,6 +7,7 @@ Features:
  * Type Checker: Disallow value transfers to contracts without a payable fallback function.
  * Type Checker: Include types in explicit conversion error message.
  * Type Checker: Raise proper error for arrays too large for ABI encoding.
+ * Type checker: Warn if using ``this`` in a constructor.
 
 Bugfixes:
  * Type Checker: Fix invalid "specify storage keyword" warning for reference members of structs.

libsolidity/analysis/StaticAnalyzer.cpp

@@ -38,12 +38,14 @@ bool StaticAnalyzer::analyze(SourceUnit const& _sourceUnit)
 bool StaticAnalyzer::visit(ContractDefinition const& _contract)
 {
 	m_library = _contract.isLibrary();
+	m_currentContract = &_contract;
 	return true;
 }
 
 void StaticAnalyzer::endVisit(ContractDefinition const&)
 {
 	m_library = false;
+	m_currentContract = nullptr;
 }
 
 bool StaticAnalyzer::visit(FunctionDefinition const& _function)
@@ -54,6 +56,7 @@ bool StaticAnalyzer::visit(FunctionDefinition const& _function)
 		solAssert(!m_currentFunction, "");
 	solAssert(m_localVarUseCount.empty(), "");
 	m_nonPayablePublic = _function.isPublic() && !_function.isPayable();
+	m_constructor = _function.isConstructor();
 	return true;
 }
 
@@ -61,6 +64,7 @@ void StaticAnalyzer::endVisit(FunctionDefinition const&)
 {
 	m_currentFunction = nullptr;
 	m_nonPayablePublic = false;
+	m_constructor = false;
 	for (auto const& var: m_localVarUseCount)
 		if (var.second == 0)
 			m_errorReporter.warning(var.first->location(), "Unused local variable");
@@ -131,6 +135,11 @@ bool StaticAnalyzer::visit(MemberAccess const& _memberAccess)
 					"\"callcode\" has been deprecated in favour of \"delegatecall\"."
 				);
 
+	if (m_constructor && m_currentContract)
+		if (ContractType const* type = dynamic_cast<ContractType const*>(_memberAccess.expression().annotation().type.get()))
+			if (type->contractDefinition() == *m_currentContract)
+				m_errorReporter.warning(_memberAccess.location(), "\"this\" used in constructor.");
+
 	return true;
 }
 

libsolidity/analysis/StaticAnalyzer.h

@@ -77,6 +77,12 @@ private:
 	std::map<VariableDeclaration const*, int> m_localVarUseCount;
 
 	FunctionDefinition const* m_currentFunction = nullptr;
+
+	/// Flag that indicates a constructor.
+	bool m_constructor = false;
+
+	/// Current contract.
+	ContractDefinition const* m_currentContract = nullptr;
 };
 
 }

test/libsolidity/SolidityNameAndTypeResolution.cpp

@@ -6373,6 +6373,20 @@ BOOST_AUTO_TEST_CASE(modifiers_access_storage_pointer)
 	CHECK_SUCCESS_NO_WARNINGS(text);
 }
 
+BOOST_AUTO_TEST_CASE(using_this_in_constructor)
+{
+	char const* text = R"(
+		contract C {
+			function C() {
+				this.f();
+			}
+			function f() {
+			}
+		}
+	)";
+	CHECK_WARNING(text, "\"this\" used in constructor");
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }