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

Group compiler warnings and errors output #9002

Closed
elenadimitrova opened this issue May 21, 2020 · 3 comments
Closed

Group compiler warnings and errors output #9002

elenadimitrova opened this issue May 21, 2020 · 3 comments

Comments

@elenadimitrova
Copy link
Collaborator

elenadimitrova commented May 21, 2020

Abstract

Solidity compiler warnings and errors are output sequentially as they are emitted, making it difficult to read. This impacts is most prominent when working on contract updates between solc versions, example below is a small chunk of the output on the argent-contracts repo when I switched part of the contracts from 0.5.4 to 0.6.8. Can you spot the ParserError?

Screenshot 2020-05-21 at 09 49 14

Motivation

Instead of outputting compiler messages sequentially in the order produced, it will be very useful for a developer working on them that these are grouped by the type of warning or error.

Specification

All warnings and errors of a certain type are grouped under a heading with the warning text, listing below the failing instances. For example all SPDX license identifier not provided in source file warnings can be grouped into 1 with that heading, listing below the failing file and line number if applicable.
To demonstrate with an abbreviated example from above:

,/Users/Elena/Source/argent-contracts/contracts/modules/ApprovedTransfer.sol: Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: <SPDX-License>" to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information.
,/Users/Elena/Source/argent-contracts/contracts/modules/CompoundManager.sol: Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: <SPDX-License>" to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information.
,/Users/Elena/Source/argent-contracts/contracts/modules/GuardianManager.sol: Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: <SPDX-License>" to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information.
,/Users/Elena/Source/argent-contracts/contracts/modules/LockManager.sol: Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: <SPDX-License>" to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information.
,/Users/Elena/Source/argent-contracts/contracts/modules/NftTransfer.sol: Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: <SPDX-License>" to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information.
,/Users/Elena/Source/argent-contracts/contracts/modules/RecoveryManager.sol: Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: <SPDX-License>" to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information.
,/Users/Elena/Source/argent-contracts/contracts/modules/SimpleUpgrader.sol: Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: <SPDX-License>" to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information.
,/Users/Elena/Source/argent-contracts/contracts/modules/TokenExchanger.sol: Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: <SPDX-License>" to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information.
,/Users/Elena/Source/argent-contracts/contracts/modules/TransferManager.sol: Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: <SPDX-License>" to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information.

becomes

Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: <SPDX-License>" to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information.
    ,/argent-contracts/contracts/modules/ApprovedTransfer.sol: 
    ,/argent-contracts/contracts/modules/CompoundManager.sol
    ,/argent-contracts/contracts/modules/GuardianManager.sol
    ,/argent-contracts/contracts/modules/LockManager.sol
    ,/argent-contracts/contracts/modules/NftTransfer.sol
    ,/argent-contracts/contracts/modules/RecoveryManager.sol
    ,/argent-contracts/contracts/modules/SimpleUpgrader.sol
    ,/argent-contracts/contracts/modules/TokenExchanger.sol
    ,/argent-contracts/contracts/modules/TransferManager.sol

Additionally, it maybe useful that error groups are reported at the top, followed by warnings as it is currently rather difficult to spot errors amongst all output. This is especially true when working on migrations where large compiler set of warnings and errors is generated.

@chriseth
Copy link
Contributor

This sounds like it could be difficult to implement when considering how IDEs use the compiler (they assume one location per error). I see that this could be useful when using solc in the commandline and when you are not really trying to fix all warnings.

Can you explain what your workflow is and why you would not just fix one reported error / warning after the other?

@elenadimitrova
Copy link
Collaborator Author

Now you said this I realise it should really be a feature of the IDE, not the compiler output, but since most devs including myself still use the command line for compilation the mindset is such that it should be provided there. I'll look into integrating this feature in the https://github.com/juanfranblanco/vscode-solidity extension perhaps. Any other ideas are welcome

@elenadimitrova
Copy link
Collaborator Author

Logged with IDE instead as it better fits there microsoft/vscode#98819

@ethereum ethereum deleted a comment from sw7240614 Jun 5, 2020
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