fix: dont strip empty () on constructors (foundry-rs#6740)

* fix: dont strip empty () on constructors

* else if

Author: mattsse

crates/fmt/src/formatter.rs

@@ -80,6 +80,13 @@ struct Context {
     if_stmt_single_line: Option<bool>,
 }
 
+impl Context {
+    /// Returns true if the current function context is the constructor
+    pub(crate) fn is_constructor_function(&self) -> bool {
+        self.function.as_ref().map_or(false, |f| matches!(f.ty, FunctionTy::Constructor))
+    }
+}
+
 /// A Solidity formatter
 #[derive(Debug)]
 pub struct Formatter<'a, W> {
@@ -3047,6 +3054,18 @@ impl<'a, W: Write> Visitor for Formatter<'a, W> {
                 self.visit_list("", args, None, Some(loc.end()), false)?
             }
             FunctionAttribute::BaseOrModifier(loc, base) => {
+                // here we need to find out if this attribute belongs to the constructor because the
+                // modifier need to include the trailing parenthesis
+                // This is very ambiguous because the modifier can either by an inherited contract
+                // or a modifier here: e.g.: This is valid constructor:
+                // `constructor() public  Ownable() OnlyOwner {}`
+                let is_constructor = self.context.is_constructor_function();
+                // we can't make any decisions here regarding trailing `()` because we'd need to
+                // find out if the `base` is a solidity modifier or an
+                // interface/contract therefor we we its raw content.
+
+                // we can however check if the contract `is` the `base`, this however also does
+                // not cover all cases
                 let is_contract_base = self.context.contract.as_ref().map_or(false, |contract| {
                     contract.base.iter().any(|contract_base| {
                         contract_base
@@ -3060,6 +3079,20 @@ impl<'a, W: Write> Visitor for Formatter<'a, W> {
 
                 if is_contract_base {
                     base.visit(self)?;
+                } else if is_constructor {
+                    // This is ambiguous because the modifier can either by an inherited
+                    // contract modifiers with empty parenthesis are
+                    // valid, but not required so we make the assumption
+                    // here that modifiers are lowercase
+                    let mut base_or_modifier =
+                        self.visit_to_chunk(loc.start(), Some(loc.end()), base)?;
+                    let is_lowercase =
+                        base_or_modifier.content.chars().next().map_or(false, |c| c.is_lowercase());
+                    if is_lowercase && base_or_modifier.content.ends_with("()") {
+                        base_or_modifier.content.truncate(base_or_modifier.content.len() - 2);
+                    }
+
+                    self.write_chunk(&base_or_modifier)?;
                 } else {
                     let mut base_or_modifier =
                         self.visit_to_chunk(loc.start(), Some(loc.end()), base)?;
@@ -3110,6 +3143,8 @@ impl<'a, W: Write> Visitor for Formatter<'a, W> {
         })?;
 
         if base.args.is_none() || base.args.as_ref().unwrap().is_empty() {
+            // This is ambiguous because the modifier can either by an inherited contract or a
+            // modifier
             if self.context.function.is_some() {
                 name.content.push_str("()");
             }

crates/fmt/testdata/ConstructorModifierStyle/fmt.sol

@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.5.2;
+
+import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
+import {ERC1155} from "solmate/tokens/ERC1155.sol";
+
+import {IAchievements} from "./interfaces/IAchievements.sol";
+import {SoulBound1155} from "./abstracts/SoulBound1155.sol";
+
+contract Achievements is IAchievements, SoulBound1155, Ownable {
+    constructor(address owner) Ownable() ERC1155() {}
+}

crates/fmt/testdata/ConstructorModifierStyle/original.sol

@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.5.2;
+
+import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
+import {ERC1155} from "solmate/tokens/ERC1155.sol";
+
+import {IAchievements} from "./interfaces/IAchievements.sol";
+import {SoulBound1155} from "./abstracts/SoulBound1155.sol";
+
+contract Achievements is IAchievements, SoulBound1155, Ownable {
+    constructor(address owner) Ownable() ERC1155() {}
+}

crates/fmt/tests/formatter.rs

@@ -181,6 +181,7 @@ macro_rules! test_directories {
 
 test_directories! {
     ConstructorDefinition,
+    ConstructorModifierStyle,
     ContractDefinition,
     DocComments,
     EnumDefinition,