Skip to content

Add no_main lint #2001

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.

Sign up for GitHub

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

Merged
jubnzv merged 11 commits into from
Nov 28, 2023
Merged

Add no_main lint #2001

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e5635b9
feat(linter): Add `no_main` lint
jubnzv Nov 11, 2023
aebee32
feat(lint): Run lint only for onchain compilation targets
jubnzv Nov 17, 2023
4033d24
chore(lint): Linting message
jubnzv Nov 17, 2023
34f236d
chore(lint): Remove the `fail` test
jubnzv Nov 17, 2023
3a4955c
chore: Add CHANGELOG entry
jubnzv Nov 20, 2023
05b3a42
fix: Remove fail test from Cargo.toml
jubnzv Nov 20, 2023
c977fa6
fix(lint): Comments, lint message
jubnzv Nov 21, 2023
bbec165
Merge remote-tracking branch 'origin/master' into 1976-no-main
jubnzv Nov 28, 2023
a1f87d9
fix(lint): Run `no_main` on e2e tests
jubnzv Nov 28, 2023
7b237d3
fix: fmt
jubnzv Nov 28, 2023
3e17b24
fix: bless tests after fmt fix
jubnzv Nov 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
feat(lint): Run lint only for onchain compilation targets 
Otherwise contracts could be built for e2e tests with the `std` feature.
  • Loading branch information
@jubnzv
jubnzv committed Nov 17, 2023
commit aebee32d9055886fd5a64c1c6ab11f387c4dff0a
13 changes: 13 additions & 0 deletions linting/src/no_main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,21 @@ declare_lint! {

declare_lint_pass!(NoMain => [NO_MAIN]);

/// Returns true if the target architecture is suitable to be executed on-chain
fn is_contract_build(cx: &EarlyContext<'_>) -> bool {
matches!(
cx.sess().target.llvm_target.to_string().as_str(),
"wasm32-unknown-unknown" | "riscv32i-unknown-none-elf"
)
}

impl EarlyLintPass for NoMain {
fn check_crate(&mut self, cx: &EarlyContext<'_>, krate: &Crate) {
// Disable when building for e2e tests
if !is_contract_build(cx) {
Copy link
Contributor Author

@jubnzv jubnzv Nov 28, 2023

Choose a reason for hiding this comment

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

Can we execute this lint on all the builds, even for e2e tests, making no_main mandatory for every ink! contract?

The point is that cargo-contract always runs linter for the host architecture. It is possible to hack it a bit, but should we? Why should we skip this lint for tests?

Copy link
Collaborator

@ascjones ascjones Nov 28, 2023

Choose a reason for hiding this comment

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

E2E tests will still run the linter because they compile the contract code blob.

Maybe what we really mean/want is that when building the contract "natively" i.e. not no_std, which happens for unit tests and metadata generation. But those are usually executed on a different path so maybe we don't need to make the distinction here.

Copy link
Contributor Author

@jubnzv jubnzv Nov 28, 2023

Choose a reason for hiding this comment

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

Yes, I see. Let's try to introduce this lint for all contracts. I think it won't make any problems.

return
}

// `no_main` is an `Inner` attribute of `#![cfg_attr(...)]`
if krate.attrs.iter().all(|attr| {
if_chain! {
Expand Down