refactor: using slices.Contains to simplify the code

[damuzhi0810]

Summary

This is a new function added in the go1.21 standard library, which can make the code more concise and easy to read.

Test Plan

[CLAassistant]

All committers have signed the CLA.

[jannotti]

Thanks, makes sense. I wonder if there's a tool to find the old code pattern. We've switched to slices.Contains in many places, but missed this one.

[damuzhi0810]

Thanks, makes sense. I wonder if there's a tool to find the old code pattern. We've switched to slices.Contains in many places, but missed this one.

I also search by keywords, and there will be a lot of omissions. But I think I can write such a tool

[jannotti]

Thanks, makes sense. I wonder if there's a tool to find the old code pattern. We've switched to slices.Contains in many places, but missed this one.

I also search by keywords, and there will be a lot of omissions. But I think I can write such a tool

I did something with semgrep and found these:

┌──────────────────┐
│ 11 Code Findings │
└──────────────────┘

    cmd/algons/dnsCmd.go
   ❯❯❱ Users.jj.no-reimplement-slices.Contains
          Don't write a loop for that, use slices.Contains instead!

          443┆ for _, known := range append(recordTypes, "") {
          444┆   if recordType == known {
          445┆           isKnown = true
          446┆           break
          447┆   }
          448┆ }

    cmd/goal/accountsList.go
   ❯❯❱ Users.jj.no-reimplement-slices.Contains
          Don't write a loop for that, use slices.Contains instead!

          104┆ for _, name := range accountList.Accounts {
          105┆   if name == accountName {
          106┆           return true
          107┆   }
          108┆ }

    data/transactions/logic/eval.go
   ❯❯❱ Users.jj.no-reimplement-slices.Contains
          Don't write a loop for that, use slices.Contains instead!

          5301┆ for _, assetID := range cx.txn.Txn.ForeignAssets {
          5302┆  if assetID == aid {
          5303┆          return true
          5304┆  }
          5305┆ }
            ⋮┆----------------------------------------
          5345┆ for _, appID := range cx.txn.Txn.ForeignApps {
          5346┆  if appID == aid {
          5347┆          return true
          5348┆  }
          5349┆ }

    network/requestTracker.go
   ❯❯❱ Users.jj.no-reimplement-slices.Contains
          Don't write a loop for that, use slices.Contains instead!

          522┆ for _, v := range []string{"localhost", "127.0.0.1",
               "[::1]", "::1", "[::]"} {
          523┆   if host == v {
          524┆           return true
          525┆   }
          526┆ }

    network/wsNetwork.go
   ❯❯❱ Users.jj.no-reimplement-slices.Contains
          Don't write a loop for that, use slices.Contains instead!

          928┆ for _, supportedProtocolVersion := range
               wn.supportedProtocolVersions {
          929┆   if supportedProtocolVersion == otherVersion {
          930┆           return supportedProtocolVersion, otherVersion
          931┆   }
          932┆ }

    network/wsPeer.go
   ❯❯❱ Users.jj.no-reimplement-slices.Contains
          Don't write a loop for that, use slices.Contains instead!

           56┆ for _, version := range SupportedProtocolVersions {
           57┆   if version == versionPeerFeatures {
           58┆           matched = true
           59┆   }
           60┆ }

    shared/pingpong/accounts.go
   ❯❯❱ Users.jj.no-reimplement-slices.Contains
          Don't write a loop for that, use slices.Contains instead!

          1006┆ for _, v := range they {
          1007┆  if v == x {
          1008┆          return they
          1009┆  }
          1010┆ }

    shared/pingpong/config.go
   ❯❯❱ Users.jj.no-reimplement-slices.Contains
          Don't write a loop for that, use slices.Contains instead!

          196┆ for _, v := range accountSampleMethods {
          197┆   if v == cfg.GeneratedAccountSampleMethod {
          198┆           sampleOk = true
          199┆           break
          200┆   }
          201┆ }

    tools/block-generator/generator/generator_ledger.go
   ❯❯❱ Users.jj.no-reimplement-slices.Contains
          Don't write a loop for that, use slices.Contains instead!

          337┆ for _, boxOptin := range ledgerBoxEvidence[appId] {
          338┆   if boxOptin == optin {
          339┆           missing = false
          340┆           break
          341┆   }
          342┆ }

    util/codecs/json.go
   ❯❯❱ Users.jj.no-reimplement-slices.Contains
          Don't write a loop for that, use slices.Contains instead!

          187┆ for _, s := range set {
          188┆   if item == s {
          189┆           return true
          190┆   }
          191┆ }

[jannotti]

Oh, the one you found can't be changed. That accountsList is a map, not a slice.

My semgrep rule had the same issue. I fixed that, so there are only 5 detected now, instead of 11.

[jannotti]

To remove approval, I have to comment. Issue is the map vs slice thing.

[damuzhi0810]

To remove approval, I have to comment. Issue is the map vs slice thing.

Great job. I'll modify the other items you found.

[damuzhi0810]

To remove approval, I have to comment. Issue is the map vs slice thing.

Everything else has been modified, I will keep an eye on ci.

But I feel like these are

    data/transactions/logic/eval.go
   ❯❯❱ Users.jj.no-reimplement-slices.Contains
          Don't write a loop for that, use slices.Contains instead!

          5301┆ for _, assetID := range cx.txn.Txn.ForeignAssets {
          5302┆  if assetID == aid {
          5303┆          return true
          5304┆  }
          5305┆ }
            ⋮┆----------------------------------------
          5345┆ for _, appID := range cx.txn.Txn.ForeignApps {
          5346┆  if appID == aid {
          5347┆          return true
          5348┆  }
          5349┆ }

Since there is no return false operation, it may not be possible to use slice.Contains optimization.

[jannotti]

But I feel like these are

    data/transactions/logic/eval.go
   ❯❯❱ Users.jj.no-reimplement-slices.Contains
          Don't write a loop for that, use slices.Contains instead!

          5301┆ for _, assetID := range cx.txn.Txn.ForeignAssets {
          5302┆  if assetID == aid {
          5303┆          return true
          5304┆  }
          5305┆ }

Since there is no return false operation, it may not be possible to use slice.Contains optimization.

	if slices.Contains(cx.txn.Txn.ForeignAssets, aid) {
		return true
	}

[jannotti]

Careful. The loop was iterating over append(recordTypes, ""). I guess blank matters here.

[jannotti]

I assume that dropping this import is why you're getting the deadlock complaints.

[jannotti]

I'd prefer swapping the return lines and removing the ! in the predicate. All else being equal, I prefer to avoid negation in predicates.