[move build] example linter: transfer-to-self

[sblackshear]

trafficstars

Description

Warn on calls of public_transfer(o, tx_context::sender()), and advise the programmer to return o from the function instead. This nudges folks toward writing compositional code that can be used in programmable transaction blocks.

We do not warn on transfer(o, tx_context::sender()), or in public_transfer(o, addr) where addr might not be tx_context_sender(). In such cases, the programmer might not be able to follow the advice of returning the object.

Test Plan

Added a transactional test that exercises the relevant behavior, but it does not print the output (will try to figure out how to do that). When I run locally, I get:

warning: Instead of transferring object of type `test::S` to `tx_context::sender()`, consider returning it from `test::public_transfer_bad1`.
            This allows a caller to use the object, and enables composability via programmable transactions
   ┌─ ./sources/test.move:15:9
   │
15 │         transfer::public_transfer(S { id: object::new(ctx), }, tx_context::sender(ctx))
   │         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: Instead of transferring object of type `test::S` to `tx_context::sender()`, consider returning it from `test::public_transfer_bad2`.
            This allows a caller to use the object, and enables composability via programmable transactions
   ┌─ ./sources/test.move:20:9
   │
20 │         transfer::public_transfer(S { id: object::new(ctx), }, sender)
   │         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

---

If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process.

Type of Change (Check all that apply)

• [ ] user-visible impact
• [ ] breaking change for a client SDKs
• [ ] breaking change for FNs (FN binary must upgrade)
• [ ] breaking change for validators or node operators (must upgrade binaries)
• [ ] breaking change for on-chain data layout
• [ ] necessitate either a data wipe or data migration

Release notes

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
offline-signer-helper	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 5, 2023 8:50am

4 Ignored Deployments

Name	Status	Preview	Comments	Updated (UTC)
explorer	⬜️ Ignored (Inspect)			Jun 5, 2023 8:50am
explorer-storybook	⬜️ Ignored (Inspect)			Jun 5, 2023 8:50am
sui-wallet-kit	⬜️ Ignored (Inspect)			Jun 5, 2023 8:50am
wallet-adapter	⬜️ Ignored (Inspect)			Jun 5, 2023 8:50am

[awelc]

I have implemented integration of the linting capability with the transactional testing framework but there are two deficiencies there that are going to be difficult to overcome without some major rewrite and I wonder how far we want to go with test support here (presumably, the most important bit is that a given message is reported for a given linting scenario, @sblackshear , @todd-mystenlabs?). Both of the deficiencies stem from the fact that the transactional framework first compiles the code and then Move model is built from the compiled modules rather than the source:

• it's tough to preserve annotations so linting suppression will does not work in test
• as far as I can tell, CompiledModule does not preserve location information, so this bit is missing in the test output

[awelc]

This has been superseded by https://github.com/MystenLabs/sui/pull/12650 (linter framework based on the Move compiler plus an example linter - warn about sharing potentially already owned objects), https://github.com/MystenLabs/sui/pull/12822 (transfer to self), and https://github.com/MystenLabs/sui/pull/12425 (unused function warning). The unused types linter was deemed to aggressive (e.g., types don't have to be instantiated to be used to parameterize dynamic field keys) and (at least for in the current form) dropped.