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

lint: limit max return values from a function to 4 #5984

Closed
p0mvn opened this issue Aug 8, 2023 · 5 comments
Closed

lint: limit max return values from a function to 4 #5984

p0mvn opened this issue Aug 8, 2023 · 5 comments
Assignees

Comments

@p0mvn
Copy link
Member

p0mvn commented Aug 8, 2023

Background

See an example of where this has grown out of control and was later fixed:
#5983

To prevent so many return values causing decrease in readability, we should add a linter

Acceptance Criteria

@p0mvn p0mvn self-assigned this Aug 8, 2023
@github-project-automation github-project-automation bot moved this to Needs Triage 🔍 in Osmosis Chain Development Aug 8, 2023
@p0mvn
Copy link
Member Author

p0mvn commented Aug 8, 2023

These examples need fixing prior to adding the linter:

x/twap/client/cli/query.go:152:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 5 (revive)
func twapQueryParseArgs(args []string) (poolId uint64, baseDenom string, startTime time.Time, endTime time.Time, err error) {
^
simulation/executor/legacyconfig.go:118:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 5 (revive)
func SetupSimulation(dirPrefix, dbName string) (cfg Config, db dbm.DB, logger log.Logger, cleanup func(), err error) {
^
x/concentrated-liquidity/math/tick.go:16:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 5 (revive)
func TicksToSqrtPrice(lowerTick, upperTick int64) (sdk.Dec, sdk.Dec, sdk.Dec, sdk.Dec, error) {
^
x/concentrated-liquidity/position.go:396:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 5 (revive)
func (k Keeper) CreateFullRangePosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddress, coins sdk.Coins) (positionId uint64, amount0, amount1 sdk.Int, liquidity sdk.Dec, err error) {
^
x/concentrated-liquidity/position.go:427:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 6 (revive)
func (k Keeper) CreateFullRangePositionLocked(ctx sdk.Context, clPoolId uint64, owner sdk.AccAddress, coins sdk.Coins, remainingLockDuration time.Duration) (positionId uint64, amount0, amount1 sdk.Int, liquidity sdk.Dec, concentratedLockID uint64, err error) {
^
x/concentrated-liquidity/position.go:447:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 6 (revive)
func (k Keeper) CreateFullRangePositionUnlocking(ctx sdk.Context, clPoolId uint64, owner sdk.AccAddress, coins sdk.Coins, remainingLockDuration time.Duration) (positionId uint64, amount0, amount1 sdk.Int, liquidity sdk.Dec, concentratedLockID uint64, err error) {
^
x/concentrated-liquidity/swaps.go:339:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 5 (revive)
func (k Keeper) computeOutAmtGivenIn(
        ctx sdk.Context,
        poolId uint64,
        tokenInMin sdk.Coin,
        tokenOutDenom string,
        spreadFactor sdk.Dec,
        priceLimit sdk.Dec,
) (tokenIn, tokenOut sdk.Coin, poolUpdates PoolUpdates, totalSpreadFactors sdk.Dec, err error) {
x/concentrated-liquidity/lp.go:35:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 7 (revive)
func (k Keeper) CreatePosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddress, tokensProvided sdk.Coins, amount0Min, amount1Min sdk.Int, lowerTick, upperTick int64) (positionId uint64, actualAmount0 sdk.Int, actualAmount1 sdk.Int, liquidityDelta sdk.Dec, lowerTickResult int64, upperTickResult int64, err error) {
^
x/concentrated-liquidity/swaps.go:468:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 5 (revive)
func (k Keeper) computeInAmtGivenOut(
        ctx sdk.Context,
        desiredTokenOut sdk.Coin,
        tokenInDenom string,
        spreadFactor sdk.Dec,
        priceLimit sdk.Dec,
        poolId uint64,
) (tokenIn, tokenOut sdk.Coin, poolUpdates PoolUpdates, totalSpreadFactors sdk.Dec, err error) {
x/concentrated-liquidity/position.go:663:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 5 (revive)
func (k Keeper) validatePositionsAndGetTotalLiquidity(ctx sdk.Context, owner sdk.AccAddress, positionIds []uint64, fullyChargedDuration time.Duration) (uint64, int64, int64, sdk.Dec, error) {
^
x/concentrated-liquidity/lp.go:407:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 5 (revive)
func (k Keeper) UpdatePosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddress, lowerTick, upperTick int64, liquidityDelta sdk.Dec, joinTime time.Time, positionId uint64) (sdk.Int, sdk.Int, bool, bool, error) {
^
x/concentrated-liquidity/simulation/sim_msgs.go:325:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 6 (revive)
func RandomPreparePoolFunc(sim *osmosimtypes.SimCtx, ctx sdk.Context, k clkeeper.Keeper) (sdk.AccAddress, sdk.Coin, sdk.Coin, uint64, sdk.Dec, error) {
^
x/concentrated-liquidity/simulation/sim_msgs.go:360:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 5 (revive)
func RandomPrepareCreatePositionFunc(sim *osmosimtypes.SimCtx, ctx sdk.Context, clPool cltypes.ConcentratedPoolExtension, poolDenoms []string) (sdk.AccAddress, sdk.Coins, int64, int64, error) {
^
x/gamm/keeper/migrate.go:20:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 7 (revive)
func (k Keeper) MigrateUnlockedPositionFromBalancerToConcentrated(ctx sdk.Context,
        sender sdk.AccAddress, sharesToMigrate sdk.Coin,
        tokenOutMins sdk.Coins,
) (positionId uint64, amount0, amount1 sdk.Int, liquidity sdk.Dec, poolIdLeaving, poolIdEntering uint64, err error) {
x/gamm/simulation/sim_msgs.go:417:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 7 (revive)
func getRandPool(k keeper.Keeper, sim *simtypes.SimCtx, ctx sdk.Context) (uint64, types.CFMMPoolI, sdk.Coin, sdk.Coin, []string, string, error) {
^
x/superfluid/keeper/migrate.go:49:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 8 (revive)
func (k Keeper) RouteLockedBalancerToConcentratedMigration(ctx sdk.Context, sender sdk.AccAddress, providedLockId int64, sharesToMigrate sdk.Coin, tokenOutMins sdk.Coins) (positionId uint64, amount0, amount1 sdk.Int, liquidity sdk.Dec, poolIdLeaving, poolIdEntering, concentratedLockId uint64, err error) {
^
x/superfluid/keeper/migrate.go:79:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 8 (revive)
func (k Keeper) migrateSuperfluidBondedBalancerToConcentrated(ctx sdk.Context,
        sender sdk.AccAddress,
        originalLockId uint64,
        sharesToMigrate sdk.Coin,
        synthDenomBeforeMigration string,
        tokenOutMins sdk.Coins,
) (positionId uint64, amount0, amount1 sdk.Int, liquidity sdk.Dec, concentratedLockId, poolIdLeaving, poolIdEntering uint64, err error) {
x/superfluid/keeper/migrate.go:134:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 8 (revive)
func (k Keeper) migrateSuperfluidUnbondingBalancerToConcentrated(ctx sdk.Context,
        sender sdk.AccAddress,
        lockId uint64,
        sharesToMigrate sdk.Coin,
        synthDenomBeforeMigration string,
        tokenOutMins sdk.Coins,
) (positionId uint64, amount0, amount1 sdk.Int, liquidity sdk.Dec, concentratedLockId, poolIdLeaving, poolIdEntering uint64, err error) {
x/superfluid/keeper/concentrated_liquidity.go:32:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 6 (revive)
func (k Keeper) addToConcentratedLiquiditySuperfluidPosition(ctx sdk.Context, sender sdk.AccAddress, positionId uint64, amount0ToAdd, amount1ToAdd sdk.Int) (uint64, sdk.Int, sdk.Int, sdk.Dec, uint64, error) {
^
x/superfluid/keeper/migrate.go:190:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 8 (revive)
func (k Keeper) migrateNonSuperfluidLockBalancerToConcentrated(ctx sdk.Context,
        sender sdk.AccAddress,
        lockId uint64,
        sharesToMigrate sdk.Coin,
        tokenOutMins sdk.Coins,
) (positionId uint64, amount0, amount1 sdk.Int, liquidity sdk.Dec, concentratedLockId, poolIdLeaving, poolIdEntering uint64, err error) {
x/superfluid/keeper/migrate.go:263:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 5 (revive)
func (k Keeper) validateMigration(ctx sdk.Context, sender sdk.AccAddress, lockId uint64, sharesToMigrate sdk.Coin) (poolIdLeaving, poolIdEntering uint64, preMigrationLock *lockuptypes.PeriodLock, remainingLockTime time.Duration, err error) {
^
tests/e2e/configurer/chain/queries.go:446:1: function-result-limit: maximum number of return results per function exceeded; max 4 but got 5 (revive)
func (n *NodeConfig) QueryPropTally(proposalNumber int) (sdk.Int, sdk.Int, sdk.Int, sdk.Int, error) {

@p0mvn
Copy link
Member Author

p0mvn commented Aug 8, 2023

Started adding linter on this branch: https://github.com/osmosis-labs/osmosis/tree/roman/max-return-linter

@ValarDragon
Copy link
Member

We should be able to have automated tooling to do these fixes as well

@ValarDragon
Copy link
Member

Can revive automatically fix these out of curiosity?

@p0mvn
Copy link
Member Author

p0mvn commented Aug 10, 2023

I was unable to find such an option. I think it would require some more complex edge case handling, something AST-like.

From doing this manually, I think it's hard to construct a deterministic set of rules that would apply to every usage of the function being refactored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

2 participants