Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

forge fmt formatting with Modifier-style base constructor call without arguments compiler error #5633

Closed
2 tasks done
vladikopl01 opened this issue Aug 15, 2023 · 3 comments · Fixed by #6740
Closed
2 tasks done
Assignees
Labels
C-forge Command: forge Cmd-forge-fmt Command: forge fmt T-bug Type: bug
Milestone

Comments

@vladikopl01
Copy link

vladikopl01 commented Aug 15, 2023

Component

Forge

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (dffdfde 2023-08-14T00:22:15.788827000Z)

What command(s) is the bug in?

forge fmt

Operating System

macOS (Apple Silicon)

Describe the bug

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() {}
}

While formatting this piece of code with forge fmt, it removes parentheses for ERC1155 inline derived constructor call. Then, the compiler gives such an error on forge build:

vladikopl@Vladichek-Air core % forge build
[⠊] Compiling...
[⠢] Compiling 1 files with 0.8.20
[⠆] Solc 0.8.20 finished in 18.36ms
Error: 
Compiler run failed:
Error (1563): Modifier-style base constructor call without arguments.
  --> contracts/Achievements.sol:17:42:
   |
17 |     constructor(address owner) Ownable() ERC1155 {}
   |                                          ^^^^^^^

May it be caused by SoulBound1155 contract as it is already derived from ERC1155? Anyways, I think formatter should recognise it, and not change ERC1155() to ERC1155

@vladikopl01 vladikopl01 added the T-bug Type: bug label Aug 15, 2023
@mattsse
Copy link
Member

mattsse commented Aug 15, 2023

likely somewhere around here @Evalir

self.surrounded(
first_surrounding,
SurroundingChunk::new(")", None, params_next_offset),
|fmt, multiline| {
let params = fmt.items_to_chunks(
params_next_offset,
func.params
.iter_mut()
.filter_map(|(loc, param)| param.as_mut().map(|param| (*loc, param))),
)?;
let after_params = if !func.attributes.is_empty() || !func.returns.is_empty() {
""
} else if func.body.is_some() {
" {"
} else {
";"
};

it's this most likely,

could be that the loc does not include the trailing (), so we perhaps need to check manually

fn visit_parameter(&mut self, parameter: &mut Parameter) -> Result<(), Self::Error> {
self.visit_source(parameter.loc)
}

@Evalir Evalir self-assigned this Aug 15, 2023
@Evalir Evalir added this to the v1.0.0 milestone Aug 15, 2023
@vladikopl01
Copy link
Author

Are there any updates? Maybe this formatting feature could be set in the config somehow?

Also, need to mention that there are some differences between forge fmt formatter and prettier-plugin-solidity

for (uint256 i = 0; i < length;) { // forge
for (uint256 i = 0; i < length; ) { // prettier

// forge
function linkThirdPartyToProfile(address _profileAddress, uint256 _thirdPartyId, uint256 _thirdPartyUserID)
    external
    onlyOperator
{
// prettier
function linkThirdPartyToProfile(
    address _profileAddress,
    uint256 _thirdPartyId,
    uint256 _thirdPartyUserID
) external onlyOperator {

// forge
associatedWallets[_primaryWallet][_associatedWallet] =
    WalletAssociation(true, primaryWallets[_primaryWallet].length);
// prettier
associatedWallets[_primaryWallet][_associatedWallet] = WalletAssociation(
    true,
    primaryWallets[_primaryWallet].length
);

@mds1 mds1 added C-forge Command: forge Cmd-forge-fmt Command: forge fmt labels Oct 24, 2023
@JoscelynFarr
Copy link

Hey guys, is there any update for this issue? After I use the command forge fmt it also removes parentheses from the constructor

img

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-forge Command: forge Cmd-forge-fmt Command: forge fmt T-bug Type: bug
Projects
No open projects
Archived in project
5 participants