fix(lint): avoid string concat in loop

[wolf31o2]

Summary by CodeRabbit

• Chores
  • Internal performance and string-construction improvements to DNS and handshake handling.
  • Reformulated internal formatting and name-decoding logic for more efficient string assembly.
  • No user-facing behavior changes, no modifications to public APIs, and no changes required from users.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR replaces direct string concatenation and fmt.Sprintf usage with strings.Builder-based accumulation in three internal functions: formatMessageAnswerSection and formatMessageQuestionSection in internal/dns/dns.go, and domainRecordNameDecode in internal/handshake/domain.go. The changes keep function signatures and control flow unchanged, alter only how strings are constructed, and do not introduce new exported entities.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Small, homogeneous refactor across two files focused on string construction.
• No control-flow, API, or behavioral intent changes; mainly implementation detail swaps.
• Quick verification of equivalence and edge cases should suffice.

Areas requiring attention:

• Confirm builder output exactly matches previous concatenation for formatMessageAnswerSection and formatMessageQuestionSection (commas, spaces, and RR string modifications).
• Verify domainRecordNameDecode handles the 0x00 terminator, pointer (0xC0) cases, and trailing dot insertion identically to prior behavior.

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The pull request title "fix(lint): avoid string concat in loop" accurately and concisely describes the main changes in the changeset. Both files in the PR replace string concatenation with strings.Builder-based approaches—in dns.go for the formatMessageAnswerSection and formatMessageQuestionSection functions, and in domain.go for the domainRecordNameDecode function. The title directly reflects this core refactoring, uses clear and specific language that communicates the primary change, and would immediately convey to a teammate reviewing the history what this PR addresses.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/lint-string-cat-loop

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6a935cd and ef91b3e.

📒 Files selected for processing (2)

• internal/dns/dns.go (1 hunks)
• internal/handshake/domain.go (4 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• internal/dns/dns.go

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Analyze (go)

🔇 Additional comments (3)

internal/handshake/domain.go (3)

16-16: LGTM! Import addition is correct.

The strings import is necessary for the strings.Builder usage and is correctly placed in alphabetical order.

---

286-286: Excellent refactoring to use strings.Builder!

The initialization and usage of strings.Builder efficiently eliminates string concatenation in the loop, which is the idiomatic Go approach for building strings incrementally.

Also applies to: 315-315

---

317-319: Correct implementation of label separator and string return.

The use of sb.Len() to conditionally insert the dot separator preserves the original DNS label assembly logic, and sb.String() correctly retrieves the final result.

Also applies to: 342-342

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (3)

internal/handshake/domain.go (1)

286-342: Good refactoring with micro-optimization opportunities.

The switch to strings.Builder correctly addresses the linting issue. Consider these performance improvements:

1. Line 315: Use sb.WriteByte(b) instead of sb.WriteString(string([]byte{b})) to avoid the byte-to-string conversion overhead.

2. Line 317: Use sb.Len() > 0 instead of len(sb.String()) > 0 to avoid creating a temporary string allocation just for the length check.

3. Line 318: Use sb.WriteByte('.') instead of sb.WriteString(".") for better performance with single characters.

Apply this diff to optimize the builder usage:

-				sb.WriteString(string([]byte{b}))
+				sb.WriteByte(b)
 			}
-			if len(sb.String()) > 0 {
-				sb.WriteString(".")
+			if sb.Len() > 0 {
+				sb.WriteByte('.')
 			}

internal/dns/dns.go (2)

554-576: Refactoring looks good; consider WriteByte for single characters.

The strings.Builder refactoring correctly addresses the linting issue. For a minor performance improvement, consider using WriteByte instead of WriteString for single-character literals on lines 556, 558, 568, 570, 572, and 574.

Example optimization:

 func formatMessageAnswerSection(section []dns.RR) string {
 	var sb strings.Builder
-	sb.WriteString("[ ")
+	sb.WriteString("[ ") // or sb.WriteByte('['); sb.WriteByte(' ')
 	for idx, rr := range section {
-		sb.WriteString("< ")
+		sb.WriteString("< ") // or sb.WriteByte('<'); sb.WriteByte(' ')
 		sb.WriteString(strings.ReplaceAll(
 			strings.TrimPrefix(
 				rr.String(),
 				";",
 			),
 			"\t",
 			" ",
 		),
 		)
-		sb.WriteString(" >")
+		sb.WriteString(" >") // or sb.WriteByte(' '); sb.WriteByte('>')
 		if idx != len(section)-1 {
-			sb.WriteString(`,`)
+			sb.WriteByte(',')
 		}
-		sb.WriteString(` `)
+		sb.WriteByte(' ')
 	}
-	sb.WriteString("]")
+	sb.WriteByte(']')
 	return sb.String()
 }

---

578-600: Refactoring looks good; consider WriteByte for single characters.

Similar to formatMessageAnswerSection, this function correctly uses strings.Builder. Consider the same micro-optimization of using WriteByte for single-character literals on lines 580, 582, 592, 594, 596, and 598.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 59260b1 and 6a935cd.

📒 Files selected for processing (2)

• internal/dns/dns.go (1 hunks)
• internal/handshake/domain.go (4 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Analyze (go)

🔇 Additional comments (1)

internal/handshake/domain.go (1)

16-16: LGTM: Import addition is correct.

The strings import is required for the strings.Builder usage in domainRecordNameDecode.

[agaffney]

Why not sb.WriteString("[ ") to save a call?

[wolf31o2]

This is more efficient than a WriteString