Skip to content
New issue

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

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

Already on GitHub? # to your account

Storage for contracts with the same name #92

Open
DanglSan opened this issue Aug 8, 2022 · 10 comments
Open

Storage for contracts with the same name #92

DanglSan opened this issue Aug 8, 2022 · 10 comments

Comments

@DanglSan
Copy link

DanglSan commented Aug 8, 2022

Running sol2uml storage -c *same name* on folder with files, containing contracts with the same name, will return storage diagram for contract with the smallest filename in sort order.

Example:

main.sol:

pragma solidity ^0.8.7;

import {A as A1} from "./Lib1.sol";
import {A as A2} from "./Lib2.sol";

contract A is A1, Math2 {
    uint32 C1 = 0;
}

Lib1.sol:

pragma solidity ^0.8.7;

contract A {
    uint32 C2 = 0;
}

Lib2.sol:

pragma solidity ^0.8.7;

contract A {
    uint32 C3 = 0;
}

In this example storage will be generated for A from Lib1.sol. But if you rename main.sol in amain.sol result will be for A from amain.sol.

Maybe it can be fixed by choosing both filename and contract name?

@naddison36
Copy link
Owner

Thanks for raising this issue. I've added a -cf, --contractFile <filename> option for storage diagrams in v2.1.5.

The contractFile can just be a filename like main.sol. Or it can be a relative filename from the working folder. eg src/contracts/main.sol.

@DanglSan
Copy link
Author

DanglSan commented Aug 9, 2022

Thx for the quick response but are you sure, it works properly? Cause I just have checked and got an error: Failed to find contract with name "A" in filename "./same_name/main.sol" for sol2uml storage ./same_name -c A -cf ./same_name/main.sol. Also tried other paths (and relative to contracts folder for -cf: sol2uml storage ./same_name -c A -cf ./main.sol) and contracts. I got this error on Windows and WSL.

@DanglSan
Copy link
Author

DanglSan commented Aug 9, 2022

UPD: Sorry, it works properly for filename, like: sol2uml storage ./same_name -c A -cf main.sol. But I still have problems with relative paths.

@DanglSan
Copy link
Author

DanglSan commented Aug 9, 2022

I think, I get it. There are some problems with path comparison, so sol2uml storage ./same_name -c A -cf same_name\main.sol works fine, while all other options such as:

  • sol2uml storage ./same_name -c A -cf ./same_name/main.sol
  • sol2uml storage ./same_name -c A -cf same_name/main.sol
  • sol2uml storage ./same_name -c A -cf .\same_name\main.sol

return an error.

@naddison36
Copy link
Owner

yeah, the relative paths has to be exact. Thinking about it some more, I can pass it through something like path.normalize to pick up your examples above.
I'll reopen this issue.

@naddison36 naddison36 reopened this Aug 9, 2022
@naddison36
Copy link
Owner

I've done a change on a branch that will hopefully work for

  • sol2uml storage ./same_name -c A -cf .\same_name\main.sol

I don't think the forward slashes will work on Windows.

You can test by installing the branch version with

npm i -g github:@naddison36/sol2uml#contract-filename

I can include this change in the next release.

@DanglSan
Copy link
Author

DanglSan commented Aug 11, 2022

I have tested the same example and it still doesn't work. Interesting is that sol2uml storage ./same_name -c A -cf .\main.sol now works fine. I am not sure, but it might be nessesary to add path.normalize in this comparison?

@naddison36
Copy link
Owner

I've added the extra path.normalize. Can you try v2.1.6

@DanglSan
Copy link
Author

DanglSan commented Aug 15, 2022

Thx, in most situations it works great now, but when I use temporary directory like C:\Users\user\AppData\Local\Temp\.tmpYLITLN for some reason relative_path here has value ..\..\..\AppData\Local\Temp\.tmpYLITLN\main.sol. And comparison of ..\..\..\AppData\Local\Temp\.tmpYLITLN\main.sol and .tmpYLITLN\main.so returns false, so I get Failed to find contract with name error again.

@naddison36
Copy link
Owner

Hmm, I'll need to look into this one. Thanks for reporting.

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

No branches or pull requests

3 participants
@naddison36 @DanglSan and others