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

Feature request: no-duplicated-import #588

Open
fedgiac opened this issue Jul 19, 2024 · 2 comments
Open

Feature request: no-duplicated-import #588

fedgiac opened this issue Jul 19, 2024 · 2 comments

Comments

@fedgiac
Copy link

fedgiac commented Jul 19, 2024

I'd like to suggest a new rule to catch when something is imported twice in a Solidity file, example below.

File ./Imported.sol:

contract Imported {}

File ./Contract.sol:

// The following line should trigger the suggested linting rule.
import {Imported, Imported} from "./Imported.sol`;

[... Solidity code that actually uses `Imported` ...]

A repeating import like the one above compiles successfully.
A repetition can also happen in different import lines and from different files, as long as the import resolves to the same object.

@dbale-altoros
Copy link
Collaborator

@fedgiac thanks a lot for posting...
we could implement that, but only in the current file
solhint does not work as a cross file checker
I'm not sure if it has too much value when checking only one file...

Is that still good for you ?

@fedgiac
Copy link
Author

fedgiac commented Jul 23, 2024

Yes!
The linting issue is always in one file: an object with the same name appears in some import statement more than once.

This is the more complicated variation of this rule that I was talking about (which happens when I merge files together and make imports work by copy-pasting).
Here I also write the content of the Imported* files to give a minimal compiling example.

File ./Imported1.sol:

contract Imported {}

File ./Imported2.sol:

import {Imported} from "./Imported1.sol";

File ./Contract.sol:

import {Imported} from "./Imported1.sol";
// The following line should trigger the suggested linting rule.
import {Imported} from "./Imported2.sol";

contract Contract is Imported {}

The code above compiles with Solidity 0.8.26.

Overall I don't think cross-file knowledge matters much: it's true that the two Imported objects could be an instance of two different objects, but in this case the Solidity compiler would return an error (seen by replacing the content of ./Imported2.sol with contract Imported {} in the example).

Another note: import renaming syntax like the following in ./Contract.sol should not trigger the linting rule, since the contract may be trying to use a different object that just happens to have the same name from another source.

import {Imported} from "./Imported1.sol";
// Does not trigger the rule. The import is renamed and used.
import {Imported as Imported2} from "./Imported2.sol";

contract Contract is Imported, Imported2 {}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants