Merge pull request #507 from iotaledger/feat/typegrouping-custom-linter

Define and use a custom linter for grouped parameter types

.github/workflows/golangci-lint.yml

@@ -6,7 +6,6 @@ on:
       - 'documentation/**'
       - 'scripts/**'
       - 'tools/**'
-
 jobs:
   golangci-lint:
     name: GolangCI-Lint
@@ -15,12 +14,26 @@ jobs:
       - name: Check out code into the Go module directory
         uses: actions/checkout@v3
 
+      - name: Checkout custom linter
+        uses: actions/checkout@v3
+        with:
+          repository: karimodm/typegroupingcheck
+          path: typegroupingcheck 
+
+      - name: Setup go
+        uses: actions/setup-go@v4
+        with:
+          go-version-file: './typegroupingcheck/go.mod'
+
+      - name: Build custom linter
+        working-directory: ./typegroupingcheck
+        run: |
+          go build -buildmode=plugin -o typegroupingcheck.so
+
       - name: golangci-lint
-        uses: reviewdog/action-golangci-lint@v2
+        uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.53.3
-          github_token: ${{ secrets.GITHUB_TOKEN }}
-          golangci_lint_flags: "--timeout=10m"
-          reporter: github-pr-check
-          filter_mode: nofilter
-          fail_on_error: true
+          skip-cache: true
+          version: latest
+          install-mode: goinstall
+          args: --timeout=10m --enable typegroupingcheck

.golangci.yml

@@ -33,12 +33,17 @@ linters-settings:
                       desc: Should be replaced with "github.com/iotaledger/hive.go/ierrors" package
                     - pkg: "github.com/pkg/errors"
                       desc: Should be replaced with "github.com/iotaledger/hive.go/ierrors" package
+    custom:
+        typegroupingcheck:
+            path: ./typegroupingcheck/typegroupingcheck.so
+            description: Check for grouped types in functions' parameters
 
 linters:
     # Disable all linters.
     disable-all: true
     # Enable specific linter
     enable:
+        #- typegroupingcheck
         - errcheck
         - gosimple
         - govet

components/metrics/collector/collector.go

@@ -44,7 +44,7 @@ func (c *Collector) Collect() {
 // Update updates the value of the existing metric defined by the subsystem and metricName.
 // Note that the label values must be passed in the same order as they were defined in the metric, and must match the
 // number of labels defined in the metric.
-func (c *Collector) Update(subsystem, metricName string, metricValue float64, labelValues ...string) {
+func (c *Collector) Update(subsystem string, metricName string, metricValue float64, labelValues ...string) {
 	m := c.getMetric(subsystem, metricName)
 	if m != nil {
 		m.update(metricValue, labelValues...)
@@ -54,15 +54,15 @@ func (c *Collector) Update(subsystem, metricName string, metricValue float64, la
 // Increment increments the value of the existing metric defined by the subsystem and metricName.
 // Note that the label values must be passed in the same order as they were defined in the metric, and must match the
 // number of labels defined in the metric.
-func (c *Collector) Increment(subsystem, metricName string, labels ...string) {
+func (c *Collector) Increment(subsystem string, metricName string, labels ...string) {
 	m := c.getMetric(subsystem, metricName)
 	if m != nil {
 		m.increment(labels...)
 	}
 }
 
 // DeleteLabels deletes the metric with the given labels values.
-func (c *Collector) DeleteLabels(subsystem, metricName string, labelValues map[string]string) {
+func (c *Collector) DeleteLabels(subsystem string, metricName string, labelValues map[string]string) {
 	m := c.getMetric(subsystem, metricName)
 	if m != nil {
 		m.deleteLabels(labelValues)
@@ -85,7 +85,7 @@ func (c *Collector) Shutdown() {
 	}
 }
 
-func (c *Collector) getMetric(subsystem, metricName string) *Metric {
+func (c *Collector) getMetric(subsystem string, metricName string) *Metric {
 	col := c.getCollection(subsystem)
 	if col != nil {
 		return col.GetMetric(metricName)

pkg/core/account/seated_accounts.go

@@ -35,7 +35,7 @@ func NewSeatedAccounts(accounts *Accounts, optMembers ...iotago.AccountID) *Seat
 		accounts:       accounts,
 		seatsByAccount: shrinkingmap.New[iotago.AccountID, SeatIndex](),
 	}
-	sort.Slice(optMembers, func(i, j int) bool {
+	sort.Slice(optMembers, func(i int, j int) bool {
 		return bytes.Compare(optMembers[i][:], optMembers[j][:]) < 0
 	})
 

pkg/protocol/block_dispatcher.go

@@ -311,7 +311,7 @@ func (b *BlockDispatcher) processWarpSyncResponse(commitmentID iotago.Commitment
 		}
 	}
 
-	blockBookedFunc := func(_, _ bool) {
+	blockBookedFunc := func(_ bool, _ bool) {
 		if bookedBlocks.Add(1) != totalBlocks {
 			return
 		}
@@ -330,7 +330,7 @@ func (b *BlockDispatcher) processWarpSyncResponse(commitmentID iotago.Commitment
 
 			targetEngine.BlockGadget.SetAccepted(block)
 
-			block.Notarized().OnUpdate(func(_, _ bool) {
+			block.Notarized().OnUpdate(func(_ bool, _ bool) {
 				// Wait for all blocks to be notarized before forcing the commitment of the slot.
 				if notarizedBlocks.Add(1) != totalBlocks {
 					return

pkg/protocol/chainmanager/manager.go

@@ -309,7 +309,7 @@ func (m *Manager) getOrCreateCommitment(id iotago.CommitmentID) (commitment *Cha
 	})
 }
 
-func (m *Manager) evaluateAgainstRootCommitment(commitment *iotago.Commitment) (isBelow, isRootCommitment bool) {
+func (m *Manager) evaluateAgainstRootCommitment(commitment *iotago.Commitment) (isBelow bool, isRootCommitment bool) {
 	isBelow = commitment.Slot <= m.rootCommitment.Commitment().Slot()
 	isRootCommitment = commitment.Equals(m.rootCommitment.Commitment().Commitment())
 

pkg/protocol/engine/blockdag/inmemoryblockdag/blockdag.go

@@ -82,15 +82,15 @@ func (b *BlockDAG) setupBlock(block *blocks.Block) {
 			return
 		}
 
-		parentBlock.Solid().OnUpdateOnce(func(_, _ bool) {
+		parentBlock.Solid().OnUpdateOnce(func(_ bool, _ bool) {
 			if unsolidParentsCount.Add(-1) == 0 {
 				if block.SetSolid() {
 					b.events.BlockSolid.Trigger(block)
 				}
 			}
 		})
 
-		parentBlock.Invalid().OnUpdateOnce(func(_, _ bool) {
+		parentBlock.Invalid().OnUpdateOnce(func(_ bool, _ bool) {
 			if block.SetInvalid() {
 				b.events.BlockInvalid.Trigger(block, ierrors.Errorf("parent block %s is marked as invalid", parent.ID))
 			}

pkg/protocol/engine/blocks/blocks.go

@@ -51,7 +51,7 @@ func (b *Blocks) Block(id iotago.BlockID) (block *Block, exists bool) {
 	return storage.Get(id)
 }
 
-func (b *Blocks) StoreOrUpdate(data *model.Block) (storedBlock *Block, evicted, updated bool) {
+func (b *Blocks) StoreOrUpdate(data *model.Block) (storedBlock *Block, evicted bool, updated bool) {
 	b.evictionMutex.RLock()
 	defer b.evictionMutex.RUnlock()
 

pkg/protocol/engine/booker/inmemorybooker/booker.go

@@ -132,7 +132,7 @@ func (b *Booker) setupBlock(block *blocks.Block) {
 			return
 		}
 
-		parentBlock.Booked().OnUpdateOnce(func(_, _ bool) {
+		parentBlock.Booked().OnUpdateOnce(func(_ bool, _ bool) {
 			if unbookedParentsCount.Add(-1) == 0 {
 				if err := b.book(block); err != nil {
 					if block.SetInvalid() {
@@ -142,7 +142,7 @@ func (b *Booker) setupBlock(block *blocks.Block) {
 			}
 		})
 
-		parentBlock.Invalid().OnUpdateOnce(func(_, _ bool) {
+		parentBlock.Invalid().OnUpdateOnce(func(_ bool, _ bool) {
 			if block.SetInvalid() {
 				b.events.BlockInvalid.Trigger(block, ierrors.New("block marked as invalid in Booker"))
 			}

pkg/protocol/engine/eviction/state.go

@@ -258,7 +258,7 @@ func (s *State) Import(reader io.ReadSeeker) error {
 	return nil
 }
 
-func (s *State) Rollback(lowerTarget, targetIndex iotago.SlotIndex) error {
+func (s *State) Rollback(lowerTarget iotago.SlotIndex, targetIndex iotago.SlotIndex) error {
 	s.evictionMutex.RLock()
 	defer s.evictionMutex.RUnlock()
 
@@ -313,7 +313,7 @@ func (s *State) setLatestNonEmptySlot(slot iotago.SlotIndex) {
 	}
 }
 
-func (s *State) activeIndexRange() (startSlot, endSlot iotago.SlotIndex) {
+func (s *State) activeIndexRange() (startSlot iotago.SlotIndex, endSlot iotago.SlotIndex) {
 	lastCommittedSlot := s.lastEvictedSlot
 	delayedSlot, valid := s.delayedBlockEvictionThreshold(lastCommittedSlot)
 

pkg/protocol/engine/mempool/conflictdag/conflictdagv1/conflict.go

@@ -137,7 +137,7 @@ func (c *Conflict[ConflictID, ResourceID, VoteRank]) JoinConflictSets(conflictSe
 		return nil, ierrors.Errorf("tried to join conflict sets of evicted conflict: %w", conflictdag.ErrEntityEvicted)
 	}
 
-	registerConflictingConflict := func(c, conflict *Conflict[ConflictID, ResourceID, VoteRank]) {
+	registerConflictingConflict := func(c *Conflict[ConflictID, ResourceID, VoteRank], conflict *Conflict[ConflictID, ResourceID, VoteRank]) {
 		c.structureMutex.Lock()
 		defer c.structureMutex.Unlock()
 
@@ -180,7 +180,7 @@ func (c *Conflict[ConflictID, ResourceID, VoteRank]) removeParent(parent *Confli
 }
 
 // UpdateParents updates the parents of the Conflict.
-func (c *Conflict[ConflictID, ResourceID, VoteRank]) UpdateParents(addedParents, removedParents ds.Set[*Conflict[ConflictID, ResourceID, VoteRank]]) (updated bool) {
+func (c *Conflict[ConflictID, ResourceID, VoteRank]) UpdateParents(addedParents ds.Set[*Conflict[ConflictID, ResourceID, VoteRank]], removedParents ds.Set[*Conflict[ConflictID, ResourceID, VoteRank]]) (updated bool) {
 	c.structureMutex.Lock()
 	defer c.structureMutex.Unlock()
 
@@ -389,7 +389,7 @@ func (c *Conflict[ConflictID, ResourceID, VoteRank]) registerChild(child *Confli
 		defer c.likedInsteadMutex.Unlock()
 
 		c.childUnhookMethods.Set(child.ID, lo.Batch(
-			c.AcceptanceStateUpdated.Hook(func(_, newState acceptance.State) {
+			c.AcceptanceStateUpdated.Hook(func(_ acceptance.State, newState acceptance.State) {
 				if newState.IsRejected() {
 					child.setAcceptanceState(newState)
 				}
@@ -432,7 +432,7 @@ func (c *Conflict[ConflictID, ResourceID, VoteRank]) unregisterChild(conflict *C
 }
 
 // addInheritedLikedInsteadReference adds the given reference as a liked instead reference from the given source.
-func (c *Conflict[ConflictID, ResourceID, VoteRank]) addInheritedLikedInsteadReference(source, reference *Conflict[ConflictID, ResourceID, VoteRank]) {
+func (c *Conflict[ConflictID, ResourceID, VoteRank]) addInheritedLikedInsteadReference(source *Conflict[ConflictID, ResourceID, VoteRank], reference *Conflict[ConflictID, ResourceID, VoteRank]) {
 	c.likedInsteadMutex.Lock()
 	defer c.likedInsteadMutex.Unlock()
 
@@ -451,7 +451,7 @@ func (c *Conflict[ConflictID, ResourceID, VoteRank]) addInheritedLikedInsteadRef
 }
 
 // removeInheritedLikedInsteadReference removes the given reference as a liked instead reference from the given source.
-func (c *Conflict[ConflictID, ResourceID, VoteRank]) removeInheritedLikedInsteadReference(source, reference *Conflict[ConflictID, ResourceID, VoteRank]) {
+func (c *Conflict[ConflictID, ResourceID, VoteRank]) removeInheritedLikedInsteadReference(source *Conflict[ConflictID, ResourceID, VoteRank], reference *Conflict[ConflictID, ResourceID, VoteRank]) {
 	c.likedInsteadMutex.Lock()
 	defer c.likedInsteadMutex.Unlock()
 

pkg/protocol/engine/mempool/conflictdag/conflictdagv1/conflictdag.go

@@ -86,7 +86,7 @@ func (c *ConflictDAG[ConflictID, ResourceID, VoteRank]) CreateConflict(id Confli
 
 			// attach to the acceptance state updated event and propagate that event to the outside.
 			// also need to remember the unhook method to properly evict the conflict.
-			c.conflictUnhooks.Set(id, newConflict.AcceptanceStateUpdated.Hook(func(oldState, newState acceptance.State) {
+			c.conflictUnhooks.Set(id, newConflict.AcceptanceStateUpdated.Hook(func(_ acceptance.State, newState acceptance.State) {
 				if newState.IsAccepted() {
 					c.events.ConflictAccepted.Trigger(newConflict.ID)
 					return
@@ -117,7 +117,6 @@ func (c *ConflictDAG[ConflictID, ResourceID, VoteRank]) UpdateConflictingResourc
 
 		return conflict.JoinConflictSets(c.conflictSets(resourceIDs))
 	}()
-
 	if err != nil {
 		return ierrors.Errorf("conflict %s failed to join conflict sets: %w", id, err)
 	}
@@ -140,7 +139,7 @@ func (c *ConflictDAG[ConflictID, ResourceID, VoteRank]) ReadConsistent(callback
 }
 
 // UpdateConflictParents updates the parents of the given Conflict and returns an error if the operation failed.
-func (c *ConflictDAG[ConflictID, ResourceID, VoteRank]) UpdateConflictParents(conflictID ConflictID, addedParentIDs, removedParentIDs ds.Set[ConflictID]) error {
+func (c *ConflictDAG[ConflictID, ResourceID, VoteRank]) UpdateConflictParents(conflictID ConflictID, addedParentIDs ds.Set[ConflictID], removedParentIDs ds.Set[ConflictID]) error {
 	newParents := ds.NewSet[ConflictID]()
 
 	updated, err := func() (bool, error) {
@@ -449,7 +448,7 @@ func (c *ConflictDAG[ConflictID, ResourceID, VoteRank]) conflictSets(resourceIDs
 }
 
 // determineVotes determines the Conflicts that are supported and revoked by the given ConflictIDs.
-func (c *ConflictDAG[ConflictID, ResourceID, VoteRank]) determineVotes(conflictIDs ds.Set[ConflictID]) (supportedConflicts, revokedConflicts ds.Set[*Conflict[ConflictID, ResourceID, VoteRank]], err error) {
+func (c *ConflictDAG[ConflictID, ResourceID, VoteRank]) determineVotes(conflictIDs ds.Set[ConflictID]) (supportedConflicts ds.Set[*Conflict[ConflictID, ResourceID, VoteRank]], revokedConflicts ds.Set[*Conflict[ConflictID, ResourceID, VoteRank]], err error) {
 	supportedConflicts = ds.NewSet[*Conflict[ConflictID, ResourceID, VoteRank]]()
 	revokedConflicts = ds.NewSet[*Conflict[ConflictID, ResourceID, VoteRank]]()
 
@@ -498,7 +497,7 @@ func (c *ConflictDAG[ConflictID, ResourceID, VoteRank]) conflictSetFactory(resou
 	return func() *ConflictSet[ConflictID, ResourceID, VoteRank] {
 		conflictSet := NewConflictSet[ConflictID, ResourceID, VoteRank](resourceID)
 
-		conflictSet.OnAllMembersEvicted(func(prevValue, newValue bool) {
+		conflictSet.OnAllMembersEvicted(func(prevValue bool, newValue bool) {
 			if newValue && !prevValue {
 				c.conflictSetsByID.Delete(conflictSet.ID)
 			}

pkg/protocol/engine/mempool/conflictdag/conflictdagv1/sorted_conflict.go

@@ -119,7 +119,7 @@ func (s *sortedConflict[ConflictID, ResourceID, VoteRank]) Unhook() {
 	}
 }
 
-func (s *sortedConflict[ConflictID, ResourceID, VoteRank]) onAcceptanceStateUpdated(_, newState acceptance.State) {
+func (s *sortedConflict[ConflictID, ResourceID, VoteRank]) onAcceptanceStateUpdated(_ acceptance.State, newState acceptance.State) {
 	if newState.IsAccepted() {
 		s.sortedSet.owner.setAcceptanceState(acceptance.Rejected)
 	}

pkg/protocol/engine/mempool/conflictdag/conflictdagv1/sorted_conflicts.go

@@ -370,7 +370,7 @@ func (s *SortedConflicts[ConflictID, ResourceID, VoteRank]) findLowerHeaviestPre
 }
 
 // swapNeighbors swaps the given members in the SortedConflicts.
-func (s *SortedConflicts[ConflictID, ResourceID, VoteRank]) swapNeighbors(heavierMember, lighterMember *sortedConflict[ConflictID, ResourceID, VoteRank]) {
+func (s *SortedConflicts[ConflictID, ResourceID, VoteRank]) swapNeighbors(heavierMember *sortedConflict[ConflictID, ResourceID, VoteRank], lighterMember *sortedConflict[ConflictID, ResourceID, VoteRank]) {
 	if heavierMember.lighterMember != nil {
 		heavierMember.lighterMember.heavierMember = lighterMember
 	}

pkg/protocol/engine/mempool/conflictdag/tests/framework.go

@@ -60,7 +60,7 @@ func (f *Framework) CreateOrUpdateConflict(alias string, resourceAliases []strin
 }
 
 // UpdateConflictParents updates the parents of the conflict with the given alias.
-func (f *Framework) UpdateConflictParents(conflictAlias string, addedParentIDs, removedParentIDs []string) error {
+func (f *Framework) UpdateConflictParents(conflictAlias string, addedParentIDs []string, removedParentIDs []string) error {
 	return f.Instance.UpdateConflictParents(f.ConflictID(conflictAlias), f.ConflictIDs(addedParentIDs...), f.ConflictIDs(removedParentIDs...))
 }
 

pkg/protocol/engine/mempool/tests/tests.go

@@ -454,7 +454,7 @@ func TestStoreAttachmentInEvictedSlot(t *testing.T, tf *TestFramework) {
 }
 
 func TestMemoryRelease(t *testing.T, tf *TestFramework) {
-	issueTransactions := func(startIndex, transactionCount int, prevStateAlias string) (int, string) {
+	issueTransactions := func(startIndex int, transactionCount int, prevStateAlias string) (int, string) {
 		index := startIndex
 		for ; index < startIndex+transactionCount; index++ {
 			signedTxAlias := fmt.Sprintf("tx%d-signed", index)

pkg/protocol/engine/mempool/v1/inclusion_flags.go

@@ -28,7 +28,7 @@ func newInclusionFlags() *inclusionFlags {
 		committedSlot: reactive.NewVariable[iotago.SlotIndex](),
 		rejected:      promise.NewEvent(),
 		// Make sure the oldest orphaned index doesn't get overridden by newer TX spending the orphaned conflict further.
-		orphanedSlot: reactive.NewVariable[iotago.SlotIndex](func(currentValue, newValue iotago.SlotIndex) iotago.SlotIndex {
+		orphanedSlot: reactive.NewVariable[iotago.SlotIndex](func(currentValue iotago.SlotIndex, newValue iotago.SlotIndex) iotago.SlotIndex {
 			if currentValue != 0 {
 				return currentValue
 			}
@@ -49,7 +49,7 @@ func (s *inclusionFlags) IsAccepted() bool {
 
 // OnAccepted registers a callback that gets triggered when the entity gets accepted.
 func (s *inclusionFlags) OnAccepted(callback func()) {
-	s.accepted.OnUpdate(func(wasAccepted, isAccepted bool) {
+	s.accepted.OnUpdate(func(wasAccepted bool, isAccepted bool) {
 		if isAccepted && !wasAccepted {
 			callback()
 		}
@@ -58,7 +58,7 @@ func (s *inclusionFlags) OnAccepted(callback func()) {
 
 // OnPending registers a callback that gets triggered when the entity gets pending.
 func (s *inclusionFlags) OnPending(callback func()) {
-	s.accepted.OnUpdate(func(wasAccepted, isAccepted bool) {
+	s.accepted.OnUpdate(func(wasAccepted bool, isAccepted bool) {
 		if !isAccepted && wasAccepted {
 			callback()
 		}
@@ -82,7 +82,7 @@ func (s *inclusionFlags) CommittedSlot() (slot iotago.SlotIndex, isCommitted boo
 
 // OnCommitted registers a callback that gets triggered when the entity gets committed.
 func (s *inclusionFlags) OnCommittedSlotUpdated(callback func(slot iotago.SlotIndex)) {
-	s.committedSlot.OnUpdate(func(_, newValue iotago.SlotIndex) {
+	s.committedSlot.OnUpdate(func(_ iotago.SlotIndex, newValue iotago.SlotIndex) {
 		callback(newValue)
 	})
 }
@@ -94,7 +94,7 @@ func (s *inclusionFlags) OrphanedSlot() (slot iotago.SlotIndex, isOrphaned bool)
 
 // OnOrphaned registers a callback that gets triggered when the entity gets orphaned.
 func (s *inclusionFlags) OnOrphanedSlotUpdated(callback func(slot iotago.SlotIndex)) {
-	s.orphanedSlot.OnUpdate(func(_, newValue iotago.SlotIndex) {
+	s.orphanedSlot.OnUpdate(func(_ iotago.SlotIndex, newValue iotago.SlotIndex) {
 		callback(newValue)
 	})
 }

pkg/protocol/engine/mempool/v1/mempool.go

@@ -240,7 +240,7 @@ func (m *MemPool[VoteRank]) Evict(slot iotago.SlotIndex) {
 	}
 }
 
-func (m *MemPool[VoteRank]) storeTransaction(signedTransaction mempool.SignedTransaction, transaction mempool.Transaction, blockID iotago.BlockID) (storedSignedTransaction *SignedTransactionMetadata, isNewSignedTransaction, isNewTransaction bool, err error) {
+func (m *MemPool[VoteRank]) storeTransaction(signedTransaction mempool.SignedTransaction, transaction mempool.Transaction, blockID iotago.BlockID) (storedSignedTransaction *SignedTransactionMetadata, isNewSignedTransaction bool, isNewTransaction bool, err error) {
 	m.evictionMutex.RLock()
 	defer m.evictionMutex.RUnlock()
 
@@ -296,7 +296,7 @@ func (m *MemPool[VoteRank]) solidifyInputs(transaction *TransactionMetadata) {
 			}
 
 			if transaction.markInputSolid() {
-				transaction.executionContext.OnUpdate(func(_, executionContext context.Context) {
+				transaction.executionContext.OnUpdate(func(_ context.Context, executionContext context.Context) {
 					m.executeTransaction(executionContext, transaction)
 				})
 			}
@@ -476,7 +476,7 @@ func (m *MemPool[VoteRank]) setupTransaction(transaction *TransactionMetadata) {
 		})
 	})
 
-	transaction.OnEarliestIncludedAttachmentUpdated(func(prevBlock, newBlock iotago.BlockID) {
+	transaction.OnEarliestIncludedAttachmentUpdated(func(prevBlock iotago.BlockID, newBlock iotago.BlockID) {
 		if err := m.updateStateDiffs(transaction, prevBlock.Slot(), newBlock.Slot()); err != nil {
 			m.errorHandler(ierrors.Wrap(err, "failed to update state diffs"))
 		}