Merge pull request #9255 from ethereum/solidity-upgrade-constructor

[BREAKING] solidity-upgrade: added module to remove visibility of constructors

docs/using-the-compiler.rst

@@ -566,33 +566,36 @@ the latest version of the compiler.
 Available upgrade modules
 ~~~~~~~~~~~~~~~~~~~~~~~~~
 
-+-----------------+---------+--------------------------------------------------+
++----------------------------+---------+--------------------------------------------------+
-| Module          | Version | Description                                      |
+| Module                     | Version | Description                                      |
-+=================+=========+==================================================+
++============================+=========+==================================================+
-| ``constructor`` | 0.5.0   | Constructors must now be defined using the       |
+| ``constructor``            | 0.5.0   | Constructors must now be defined using the       |
-|                 |         | ``constructor`` keyword.                         |
+|                            |         | ``constructor`` keyword.                         |
-+-----------------+---------+--------------------------------------------------+
++----------------------------+---------+--------------------------------------------------+
-| ``visibility``  | 0.5.0   | Explicit function visibility is now mandatory,   |
+| ``visibility``             | 0.5.0   | Explicit function visibility is now mandatory,   |
-|                 |         | defaults to ``public``.                          |
+|                            |         | defaults to ``public``.                          |
-+-----------------+---------+--------------------------------------------------+
++----------------------------+---------+--------------------------------------------------+
-| ``abstract``    | 0.6.0   | The keyword ``abstract`` has to be used if a     |
+| ``abstract``               | 0.6.0   | The keyword ``abstract`` has to be used if a     |
-|                 |         | contract does not implement all its functions.   |
+|                            |         | contract does not implement all its functions.   |
-+-----------------+---------+--------------------------------------------------+
++----------------------------+---------+--------------------------------------------------+
-| ``virtual``     | 0.6.0   | Functions without implementation outside an      |
+| ``virtual``                | 0.6.0   | Functions without implementation outside an      |
-|                 |         | interface have to be marked ``virtual``.         |
+|                            |         | interface have to be marked ``virtual``.         |
-+-----------------+---------+--------------------------------------------------+
++----------------------------+---------+--------------------------------------------------+
-| ``override``    | 0.6.0   | When overriding a function or modifier, the new  |
+| ``override``               | 0.6.0   | When overriding a function or modifier, the new  |
-|                 |         | keyword ``override`` must be used.               |
+|                            |         | keyword ``override`` must be used.               |
-+-----------------+---------+--------------------------------------------------+
++----------------------------+---------+--------------------------------------------------+
-| ``dotsyntax``   | 0.7.0   | The following syntax is deprecated:              |
+| ``dotsyntax``              | 0.7.0   | The following syntax is deprecated:              |
-|                 |         | ``f.gas(...)()``, ``f.value(...)()`` and         |
+|                            |         | ``f.gas(...)()``, ``f.value(...)()`` and         |
-|                 |         | ``(new C).value(...)()``. Replace these calls by |
+|                            |         | ``(new C).value(...)()``. Replace these calls by |
-|                 |         | ``f{gas: ..., value: ...}()`` and                |
+|                            |         | ``f{gas: ..., value: ...}()`` and                |
-|                 |         | ``(new C){value: ...}()``.                       |
+|                            |         | ``(new C){value: ...}()``.                       |
-+-----------------+---------+--------------------------------------------------+
++----------------------------+---------+--------------------------------------------------+
-| ``now``         | 0.7.0   | The ``now`` keyword is deprecated. Use           |
+| ``now``                    | 0.7.0   | The ``now`` keyword is deprecated. Use           |
-|                 |         | ``block.timestamp`` instead.                     |
+|                            |         | ``block.timestamp`` instead.                     |
-+-----------------+---------+--------------------------------------------------+
++----------------------------+---------+--------------------------------------------------+
+| ``constructor-visibility`` | 0.7.0   | Removes visibility of constructors.              |
+|                            |         |                                                  |
++----------------------------+---------+--------------------------------------------------+
 
 Please read :doc:`0.5.0 release notes <050-breaking-changes>`,
 :doc:`0.6.0 release notes <060-breaking-changes>` and
@@ -632,116 +635,88 @@ If you found a bug or if you have a feature request, please
 Example
 ~~~~~~~
 
-Assume you have the following contracts you want to update declared in ``Source.sol``:
+Assume that you have the following contract in ``Source.sol``:
 
-.. code-block:: none
+.. code-block:: solidity
 
-    // This will not compile after 0.5.0
+    pragma solidity >=0.6.0 <0.6.4;
+    // This will not compile after 0.7.0
     // SPDX-License-Identifier: GPL-3.0
-    pragma solidity >0.4.23 <0.5.0;
+    contract C {
-
+        // FIXME: remove constructor visibility and make the contract abstract
-    contract Updateable {
+        constructor() internal {}
-        function run() public view returns (bool);
-        function update() public;
     }
 
-    contract Upgradable {
+    contract D {
-        function run() public view returns (bool);
+        uint time;
-        function upgrade();
+
+        function f() public payable {
+            // FIXME: change now to block.timestamp
+            time = now;
+        }
     }
 
-    contract Source is Updateable, Upgradable {
+    contract E {
-        function Source() public {}
+        D d;
 
-        function run()
+        // FIXME: remove constructor visibility
-            public
+        constructor() public {}
-            view
-            returns (bool) {}
 
-        function update() {}
+        function g() public {
-        function upgrade() {}
+            // FIXME: change .value(5) =>  {value: 5}
+            d.f.value(5)();
+        }
     }
 
 
+
 Required changes
 ^^^^^^^^^^^^^^^^
 
-To bring the contracts up to date with the current Solidity version, the
+The above contract will not compile starting from 0.7.0. To bring the contract up to date with the
-following upgrade modules have to be executed: ``constructor``,
+current Solidity version, the following upgrade modules have to be executed:
-``visibility``, ``abstract``, ``override`` and ``virtual``. Please read the
+``constructor-visibility``, ``now`` and ``dotsyntax``. Please read the documentation on
-documentation on :ref:`available modules <upgrade-modules>` for further details.
+:ref:`available modules <upgrade-modules>` for further details.
+
 
 Running the upgrade
 ^^^^^^^^^^^^^^^^^^^
 
-In this example, all modules needed to upgrade the contracts above,
+It is recommended to explicitly specify the upgrade modules by using ``--modules`` argument.
-are available and all of them are activated by default. Therefore you
-do not need to specify the ``--modules`` option.
 
 .. code-block:: none
 
-    $ solidity-upgrade Source.sol --dry-run
+   $ solidity-upgrade --modules constructor-visibility,now,dotsyntax Source.sol
 
-.. code-block:: none
+The command above applies all changes as shown below. Please review them carefully (the pragmas will
-
+have to be updated manually.)
-    Running analysis (and upgrade) on given source files.
-    ..............
-
-    After upgrade:
-
-    Found 0 errors.
-    Found 0 upgrades.
-
-The above performs a dry-run upgrade on the given file and logs statistics at
-the end. In this case, the upgrade was successful and no further adjustments are
-needed.
-
-Finally, you can run the upgrade and also write to the source file.
-
-.. code-block:: none
-
-    $ solidity-upgrade Source.sol
-
-.. code-block:: none
-
-    Running analysis (and upgrade) on given source files.
-    ..............
-
-    After upgrade:
-
-    Found 0 errors.
-    Found 0 upgrades.
-
-
-Review changes
-^^^^^^^^^^^^^^
-
-The command above applies all changes as shown below. Please review them carefully.
 
 .. code-block:: solidity
 
-    // SPDX-License-Identifier: GPL-3.0
     pragma solidity >0.6.99 <0.8.0;
-
+    // SPDX-License-Identifier: GPL-3.0
-    abstract contract Updateable {
+    abstract contract C {
-        function run() public view virtual returns (bool);
+        // FIXME: remove constructor visibility and make the contract abstract
-        function update() public virtual;
+        constructor() {}
     }
 
-    abstract contract Upgradable {
+    contract D {
-        function run() public view virtual returns (bool);
+        uint time;
-        function upgrade() public virtual;
+
+        function f() public payable {
+            // FIXME: change now to block.timestamp
+            time = block.timestamp;
+        }
     }
 
-    contract Source is Updateable, Upgradable {
+    contract E {
+        D d;
+
+        // FIXME: remove constructor visibility
         constructor() {}
 
-        function run()
+        function g() public {
-            public
+            // FIXME: change .value(5) =>  {value: 5}
-            view
+            d.f{value: 5}();
-            override(Updateable,Upgradable)
+        }
-            returns (bool) {}
-
-        function update() public override {}
-        function upgrade() public override {}
     }

tools/solidityUpgrade/SourceTransform.h

@@ -133,7 +133,8 @@ public:
 			return regex_replace(
 				_location.text(),
 				_regex,
-				_expression + " " + _keyword
+				_expression + " " + _keyword,
+				std::regex_constants::format_first_only
 			);
 		else
 			solAssert(
@@ -275,6 +276,14 @@ public:
 	{
 		return regex_replace(_location.text(), std::regex{"now"}, "block.timestamp");
 	}
+
+	static std::string removeVisibility(langutil::SourceLocation const& _location)
+	{
+		std::string replacement = _location.text();
+		for (auto const& replace: {"public ", "public", "internal ", "internal", "external ", "external"})
+			replacement = regex_replace(replacement, std::regex{replace}, "");
+		return replacement;
+	}
 };
 
 }

tools/solidityUpgrade/SourceUpgrade.cpp

@@ -199,6 +199,8 @@ Allowed options)",
 				m_suite.activateModule(Module::DotSyntax);
 			else if (module == "now")
 				m_suite.activateModule(Module::NowKeyword);
+			else if (module == "constructor-visibility")
+				m_suite.activateModule(Module::ConstrutorVisibility);
 			else
 			{
 				error() << "Unknown upgrade module \"" + module + "\"" << endl;

tools/solidityUpgrade/SourceUpgrade.h

@@ -60,7 +60,8 @@ private:
 		OverridingFunction,
 		VirtualFunction,
 		DotSyntax,
-		NowKeyword
+		NowKeyword,
+		ConstrutorVisibility
 	};
 
 	/// Upgrade suite that hosts all available modules.
@@ -88,6 +89,8 @@ private:
 				DotSyntax{m_changes}.analyze(_sourceUnit);
 			if (isActivated(Module::NowKeyword))
 				NowKeyword{m_changes}.analyze(_sourceUnit);
+			if (isActivated(Module::ConstrutorVisibility))
+				ConstructorVisibility{m_changes}.analyze(_sourceUnit);
 		}
 
 		void activateModule(Module _module) { m_modules.insert(_module); }
@@ -108,7 +111,8 @@ private:
 			Module::OverridingFunction,
 			Module::VirtualFunction,
 			Module::DotSyntax,
-			Module::NowKeyword
+			Module::NowKeyword,
+			Module::ConstrutorVisibility
 		};
 	};
 

tools/solidityUpgrade/Upgrade070.cpp

@@ -59,3 +59,27 @@ void NowKeyword::endVisit(Identifier const& _identifier)
 			);
 		}
 }
+
+void ConstructorVisibility::endVisit(ContractDefinition const& _contract)
+{
+	if (!_contract.abstract())
+		for (FunctionDefinition const* function: _contract.definedFunctions())
+			if (
+				function->isConstructor() &&
+				!function->noVisibilitySpecified() &&
+				function->visibility() == Visibility::Internal
+			)
+				m_changes.emplace_back(
+					UpgradeChange::Level::Safe,
+					_contract.location(),
+					SourceTransform::insertBeforeKeyword(_contract.location(), "contract", "abstract")
+				);
+
+	for (FunctionDefinition const* function: _contract.definedFunctions())
+		if (function->isConstructor() && !function->noVisibilitySpecified())
+			m_changes.emplace_back(
+				UpgradeChange::Level::Safe,
+				function->location(),
+				SourceTransform::removeVisibility(function->location())
+			);
+}

tools/solidityUpgrade/Upgrade070.h

@@ -41,4 +41,13 @@ private:
 	void endVisit(frontend::Identifier const& _expression) override;
 };
 
+class ConstructorVisibility: public AnalysisUpgrade
+{
+public:
+	using AnalysisUpgrade::AnalysisUpgrade;
+	void analyze(frontend::SourceUnit const& _sourceUnit) { _sourceUnit.accept(*this); }
+private:
+	void endVisit(frontend::ContractDefinition const& _contract) override;
+};
+
 }