Skip to content

refactor: eliminate duplicate code in DIFC labels and RPC logger#2006

Merged
lpcox merged 3 commits intomainfrom
copilot/duplicate-code-analysis
Mar 16, 2026
Merged

refactor: eliminate duplicate code in DIFC labels and RPC logger#2006
lpcox merged 3 commits intomainfrom
copilot/duplicate-code-analysis

Conversation

Copy link
Contributor

Copilot AI commented Mar 16, 2026

Closes #2001, closes #2002, closes #2003

Summary

Addresses the duplicate-code analysis report by refactoring two patterns identified in issues #2002 and #2003.

internal/difc/labels.go (issue #2002)

Added a getLabel() *Label helper method to both SecrecyLabel and IntegrityLabel. The helper safely handles nil receivers and returns nil in that case. This eliminates four identical 6-line nil-guard extraction blocks that preceded every call to checkFlowHelper, and simplifies both Clone() methods.

  • CanFlowTo and CheckFlow on both types now call l.getLabel() / target.getLabel() directly — the nil-unwrapping blocks are gone
  • Clone() on both types uses l.getLabel() == nil to guard the early-return path (covers both nil-receiver and nil-Label cases)

internal/logger/rpc_logger.go (issue #2003)

Extracted an unexported newRPCMessageInfo constructor that builds an RPCMessageInfo and sets the Error field when the supplied error is non-nil. logRPCMessageToAll now calls this helper twice (once with MaxPayloadPreviewLengthText, once with MaxPayloadPreviewLengthMarkdown) instead of repeating the struct literal + if err != nil block.

Impact

  • ~21 net lines removed (95 → 74 in labels.go region; 38 → 22 in rpc_logger.go region)
  • No behaviour change; all existing tests pass (make agent-finished ✓)
  • Future additions to RPCMessageInfo fields or nil-guard logic now have a single place to update

Copilot AI and others added 2 commits March 16, 2026 04:09
…, #2002, #2003)

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot AI requested a review from lpcox March 16, 2026 04:14
@lpcox lpcox marked this pull request as ready for review March 16, 2026 04:17
Copilot AI review requested due to automatic review settings March 16, 2026 04:17
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors two previously duplicated patterns to reduce boilerplate and centralize logic in DIFC label flow checks and RPC message logging, aligning with the duplicate-code analysis issues (#2002, #2003).

Changes:

  • Added getLabel() *Label helpers on SecrecyLabel and IntegrityLabel to remove repeated nil-unwrapping blocks and simplify Clone().
  • Extracted newRPCMessageInfo helper to avoid duplicated RPCMessageInfo construction in logRPCMessageToAll.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
internal/difc/labels.go Adds getLabel() helpers and refactors flow-check and clone methods to use them.
internal/logger/rpc_logger.go Adds newRPCMessageInfo constructor and uses it to build text/markdown log payloads without duplication.
Comments suppressed due to low confidence (1)

internal/difc/labels.go:310

  • Same as the SecrecyLabel version: this comment implies nil is only possible when the receiver is nil, but l.Label may also be nil. Please adjust the comment to mention both cases to keep docs accurate.
// getLabel returns the underlying Label, or nil if the receiver is nil.
func (l *IntegrityLabel) getLabel() *Label {
	if l == nil {
		return nil
	}
	return l.Label

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants