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

Linter: Add links to detailed lint description #2170

Merged
merged 3 commits into from
Mar 21, 2024

Conversation

cmichi
Copy link
Collaborator

@cmichi cmichi commented Mar 20, 2024

Summary

  • [n] y/n | Does it introduce breaking changes?
  • [y] y/n | Is it dependant on the specific version of cargo-contract or pallet-contracts?

I'll create an accompanying cargo-contract PR so that the new version of these lints is used.

Description

Closes #2169.

For the linter warnings, I've added a link to the detailed description of the lint on use.ink.

image

Checklist before requesting a review

  • My code follows the style guidelines of this project
  • I have added an entry to CHANGELOG.md
  • I have added tests that prove my fix is effective or that my feature works
  • Any dependent changes have been merged and published in downstream modules

@cmichi cmichi requested a review from smiasojed March 20, 2024 18:14
@cmichi cmichi requested a review from jubnzv March 20, 2024 18:19
@cmichi cmichi marked this pull request as ready for review March 20, 2024 18:19
@codecov-commenter
Copy link

codecov-commenter commented Mar 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 54.21%. Comparing base (0e84193) to head (87c5787).
Report is 8 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2170      +/-   ##
==========================================
+ Coverage   53.65%   54.21%   +0.56%     
==========================================
  Files         225      231       +6     
  Lines        7095     7204     +109     
  Branches     3129     3153      +24     
==========================================
+ Hits         3807     3906      +99     
- Misses       3288     3298      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

🦑 📈 ink! Example Contracts ‒ Changes Report 📉 🦑

These are the results when building the integration-tests/* contracts from this branch with cargo-contract and comparing them to ink! master:

Contract Upstream Size (kB) PR Size (kB) Diff (kB) Diff (%) Change
call-builder-return-value 9.249 9.249 0 0
call-runtime 2.071 2.071 0 0
combined-extension 2.132 2.132 0 0
conditional-compilation 1.502 1.502 0 0
contract-storage 7.58 7.58 0 0
contract-terminate 1.369 1.369 0 0
contract-transfer 1.731 1.731 0 0
cross-contract-calls 7.732 7.732 0 0
cross-contract-calls/other-contract 1.595 1.595 0 0
custom-allocator 7.787 7.787 0 0
custom-environment 2.158 2.158 0 0
dns 7.355 7.355 0 0
e2e-call-runtime 1.32 1.32 0 0
e2e-runtime-only-backend 1.901 1.901 0 0
erc1155 14.345 14.345 0 0
erc20 6.955 6.955 0 0
erc721 10.044 10.044 0 0
events 5.27 5.27 0 0
flipper 1.651 1.651 0 0
incrementer 1.516 1.516 0 0
lang-err-integration-tests/call-builder-delegate 2.65 2.65 0 0
lang-err-integration-tests/call-builder 5.571 5.571 0 0
lang-err-integration-tests/constructors-return-value 1.997 1.997 0 0
lang-err-integration-tests/contract-ref 5.062 5.062 0 0
lang-err-integration-tests/integration-flipper 1.827 1.827 0 0
lazyvec-integration-test 4.66 4.66 0 0
mapping-integration-tests 8.036 8.036 0 0
mother 12.753 12.753 0 0
multi-contract-caller 6.654 6.654 0 0
multi-contract-caller/accumulator 1.388 1.388 0 0
multi-contract-caller/adder 1.922 1.922 0 0
multi-contract-caller/subber 1.942 1.942 0 0
multisig 21.871 21.871 0 0
payment-channel 5.742 5.742 0 0
psp22-extension 7.083 7.083 0 0
rand-extension 2.977 2.977 0 0
sr25519-verification 1.154 1.154 0 0
static-buffer 2.578 2.578 0 0
trait-dyn-cross-contract-calls 2.899 2.899 0 0
trait-dyn-cross-contract-calls/contracts/incrementer 1.557 1.557 0 0
trait-erc20 7.331 7.331 0 0
trait-flipper 1.502 1.502 0 0
trait-incrementer 1.626 1.626 0 0
upgradeable-contracts/delegator 3.96 3.96 0 0
upgradeable-contracts/delegator/delegatee 1.641 1.641 0 0
upgradeable-contracts/delegator/delegatee2 1.641 1.641 0 0
upgradeable-contracts/set-code-hash-migration 1.755 1.755 0 0
upgradeable-contracts/set-code-hash-migration/migration 1.462 1.462 0 0
upgradeable-contracts/set-code-hash-migration/updated-incrementer 1.909 1.909 0 0
upgradeable-contracts/set-code-hash 1.755 1.755 0 0
upgradeable-contracts/set-code-hash/updated-incrementer 1.733 1.733 0 0
wildcard-selector 2.858 2.858 0 0

Link to the run | Last update: Wed Mar 20 20:38:09 CET 2024

Copy link
Member

@jubnzv jubnzv left a comment

Choose a reason for hiding this comment

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

What do you think about implementing a more generic approach similar to clippy?

This could be achievable by adding a function like docs_link and creating ctx.tcx.node_span_lint wrappers like this one.

@cmichi
Copy link
Collaborator Author

cmichi commented Mar 21, 2024

What do you think about implementing a more generic approach similar to clippy?

That sounds good! I'd merge this to have something in right now and create a follow-up ticket.

@cmichi cmichi merged commit bdefc46 into master Mar 21, 2024
23 checks passed
@cmichi cmichi deleted the cmichi-add-links-to-lint-messages branch March 21, 2024 12:03
@cmichi cmichi mentioned this pull request Nov 28, 2024
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

Successfully merging this pull request may close these issues.

Add links to linter warning details on use.ink
4 participants