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

Truffle support #393

Merged
merged 18 commits into from
Apr 19, 2023
Merged

Truffle support #393

merged 18 commits into from
Apr 19, 2023

Conversation

antico5
Copy link
Collaborator

@antico5 antico5 commented Feb 14, 2023

  • General
    • Identify projects that have truffle.js or truffle-config.js
    • Config file should be loadable (no errors) for validation to work. Same as hardhat
  • Config
    • Solc version (semver format supported. i.e. ^0.8.0
    • Contracts directory
  • Import resolution
    • Absolute path
    • Relative path
    • Truffle direct imports (i.e. truffle/console.sol)
    • Local node modules contracts
    • Global node modules contracts

Closes #45

@codecov-commenter
Copy link

codecov-commenter commented Feb 14, 2023

Codecov Report

❗ No coverage uploaded for pull request base (development@1dd334e). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 4917f2d differs from pull request most recent head ed29af5. Consider uploading reports for the commit ed29af5 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@              Coverage Diff               @@
##             development     #393   +/-   ##
==============================================
  Coverage               ?   54.28%           
==============================================
  Files                  ?      183           
  Lines                  ?     4681           
  Branches               ?      767           
==============================================
  Hits                   ?     2541           
  Misses                 ?     1905           
  Partials               ?      235           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@kanej
Copy link
Member

kanej commented Mar 13, 2023

I have been running through the truffle getting started guide:

https://trufflesuite.com/docs/truffle/quickstart/

After unboxing metacoin, I have a working project that I open with the extension:

truffle unbox metacoin [PATH/TO/DIRECTORY]

On opening the ./test/TestMetaCoin.sol file I get red squiggly on direct truffle imports:

image

Specifically:

image

@antico5
Copy link
Collaborator Author

antico5 commented Mar 15, 2023

@kanej the truffle/Assert.sol import should be working. Can you verify that the contract is being identified as a truffle project from within the extension? also if there's anything wrong in the logs. The way it's resolved is by looking into either a local or global installation of truffle. For the DeployedAddresses.sol import, it won't work because it's a source generated dynamically by truffle. But truffle console, asserts, etc imports it should work.

@kanej
Copy link
Member

kanej commented Apr 13, 2023

@kanej the truffle/Assert.sol import should be working. Can you verify that the contract is being identified as a truffle project from within the extension? also if there's anything wrong in the logs. The way it's resolved is by looking into either a local or global installation of truffle. For the DeployedAddresses.sol import, it won't work because it's a source generated dynamically by truffle. But truffle console, asserts, etc imports it should work.

What do you think of suppressing an error on the import of truffle/DeployedAddresses.sol?

@antico5
Copy link
Collaborator Author

antico5 commented Apr 13, 2023

@kanej the truffle/Assert.sol import should be working. Can you verify that the contract is being identified as a truffle project from within the extension? also if there's anything wrong in the logs. The way it's resolved is by looking into either a local or global installation of truffle. For the DeployedAddresses.sol import, it won't work because it's a source generated dynamically by truffle. But truffle console, asserts, etc imports it should work.

What do you think of suppressing an error on the import of truffle/DeployedAddresses.sol?

I think it works. We can add more to that list if we discover there are other imports of this kind.

@antico5
Copy link
Collaborator Author

antico5 commented Apr 13, 2023

@kanej the truffle/Assert.sol import should be working. Can you verify that the contract is being identified as a truffle project from within the extension? also if there's anything wrong in the logs. The way it's resolved is by looking into either a local or global installation of truffle. For the DeployedAddresses.sol import, it won't work because it's a source generated dynamically by truffle. But truffle console, asserts, etc imports it should work.

What do you think of suppressing an error on the import of truffle/DeployedAddresses.sol?

I tried this and actually found out that it doesn't quite work. I tried 2 options: 1st, suppressing the error, and 2nd, injecting a source with empty content for DeployedAddresses.sol. In the first case, all validation is lost because solc doesn't return other errors if there are import errors present, and in the 2nd case, the import line is not erroring but the usages e.g. DeployedAddresses.MetaCoin() will error because there's no such identifier.

@kanej
Copy link
Member

kanej commented Apr 13, 2023

@antico5 is there an approach to resolving the truffle contracts under node modules as part of the truffle project?

The particular problem I am thinking of is from a truffle solidity test, jumping into the assert sol file via import "truffle/Assert.sol";, the file shows up as having project none, which I think leads to the imports not resolving.

image

return "Truffle";
}

public async initialize(): Promise<void> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we trigger a reinitialization on each config change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that logic is on the onWatchedFilesChanges method of this class.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed a change to get to the comparison check on onWatchedFilesChanges as the uri had file:// as a prefix.

Is there a better way of doing that comparison with inbuilt vscode functions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use URI.parse(<uri>).fsPath. The import is import { URI } from "vscode-uri";

@antico5
Copy link
Collaborator Author

antico5 commented Apr 14, 2023

@antico5 is there an approach to resolving the truffle contracts under node modules as part of the truffle project?

The particular problem I am thinking of is from a truffle solidity test, jumping into the assert sol file via import "truffle/Assert.sol";, the file shows up as having project none, which I think leads to the imports not resolving.

image

We could easily associate files under node_modules to the truffle project, although that wouldn't work if truffle is being used as a global package. For the global package, I don't know how we could associate them with a single project, since many open projects could be using the global module.

@kanej
Copy link
Member

kanej commented Apr 17, 2023

@antico5 is there an approach to resolving the truffle contracts under node modules as part of the truffle project?
The particular problem I am thinking of is from a truffle solidity test, jumping into the assert sol file via import "truffle/Assert.sol";, the file shows up as having project none, which I think leads to the imports not resolving.
image

We could easily associate files under node_modules to the truffle project, although that wouldn't work if truffle is being used as a global package. For the global package, I don't know how we could associate them with a single project, since many open projects could be using the global module.

Lets take that approach. I think we can assume that a global install is less common and just deal with local node_modules.

@antico5
Copy link
Collaborator Author

antico5 commented Apr 17, 2023

@antico5 is there an approach to resolving the truffle contracts under node modules as part of the truffle project?
The particular problem I am thinking of is from a truffle solidity test, jumping into the assert sol file via import "truffle/Assert.sol";, the file shows up as having project none, which I think leads to the imports not resolving.
image

We could easily associate files under node_modules to the truffle project, although that wouldn't work if truffle is being used as a global package. For the global package, I don't know how we could associate them with a single project, since many open projects could be using the global module.

Lets take that approach. I think we can assume that a global install is less common and just deal with local node_modules.

Done!

Copy link
Member

@kanej kanej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested on the latest getting started guide for truffle and we are providing a significantly improved experience.

We should hold off on merging until we have a release schedule (and consideration of docs)

@antico5 antico5 merged commit c42fa62 into development Apr 19, 2023
@antico5 antico5 deleted the truffle_support branch April 19, 2023 16:52
kanej added a commit that referenced this pull request Apr 26, 2023
* feat: server document formatting provider

* tests for server document formatting

* truffle adapter scaffolding

* wip import resolution

* support node modules imports

* truffle validation

* read truffle config

* reinitialize truffle project on config change

* read globalNodeModulesPath only on initialization

* change crawlDependencies to use Set for visited nodes

* tests for truffle project

* update TruffleProject#fileBelongs

* add support for DeployedAddresses.sol

* feat: base DeployedAddresses off of build dir

* test: add failed initialization for truffle

* fix: reinitialize on change to truffle config

* claim contracts under node_modules on truffle project

* use vscode-uri to get path from uri

---------

Co-authored-by: John Kane <john@kanej.me>
kanej added a commit that referenced this pull request Apr 26, 2023
* feat: server document formatting provider

* tests for server document formatting

* truffle adapter scaffolding

* wip import resolution

* support node modules imports

* truffle validation

* read truffle config

* reinitialize truffle project on config change

* read globalNodeModulesPath only on initialization

* change crawlDependencies to use Set for visited nodes

* tests for truffle project

* update TruffleProject#fileBelongs

* add support for DeployedAddresses.sol

* feat: base DeployedAddresses off of build dir

* test: add failed initialization for truffle

* fix: reinitialize on change to truffle config

* claim contracts under node_modules on truffle project

* use vscode-uri to get path from uri

---------

Co-authored-by: John Kane <john@kanej.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add Truffle adapter
3 participants