From 10dc4ed7b0f04d6844778ebdb51238b016d76f13 Mon Sep 17 00:00:00 2001 From: "Iskander (Alex) Sharipov" Date: Thu, 30 Dec 2021 12:05:21 +0300 Subject: [PATCH] ruleguard: better text truncation in the rendered message (#330) Instead of using variable names like `$x` when substitution string is too long, truncate it to the max len. So "some very long text" becomes "some v<...> text". This is more user-friendly, as pattern variables are very non-intuitive in most cases. --- analyzer/testdata/src/extra/file.go | 2 +- ruleguard/ruleguard_test.go | 36 +++++++++++++++++++++++++++++ ruleguard/runner.go | 22 +++++++++++++----- 3 files changed, 53 insertions(+), 7 deletions(-) diff --git a/analyzer/testdata/src/extra/file.go b/analyzer/testdata/src/extra/file.go index b977437e..f313a736 100644 --- a/analyzer/testdata/src/extra/file.go +++ b/analyzer/testdata/src/extra/file.go @@ -328,7 +328,7 @@ func testYodaExpr() { if nil != clusterContext.PostInstallData.CoreDNSUpdateFunction { // want `\Qsuggestion: clusterContext.PostInstallData.CoreDNSUpdateFunction != nil` } // This is far too long, so it's shortened in the output. - if nil != clusterContext.PostInstallData.AnotherNestedStruct.DeeplyNestedField { // want `\Qsuggestion: $s != nil` + if nil != clusterContext.PostInstallData.AnotherNestedStruct.DeeplyNestedField { // want `\QclusterContext.PostInstallD<...>stedStruct.DeeplyNestedField != nil` } } diff --git a/ruleguard/ruleguard_test.go b/ruleguard/ruleguard_test.go index e32d7a24..6b47cb25 100644 --- a/ruleguard/ruleguard_test.go +++ b/ruleguard/ruleguard_test.go @@ -9,6 +9,42 @@ import ( "github.com/quasilyte/gogrep" ) +func TestTruncateText(t *testing.T) { + tests := []struct { + input string + maxLen int + want string + }{ + {"hello world", 60, "hello world"}, + {"hello world", 8, "h<...>ld"}, + {"hello world", 7, "h<...>d"}, + {"hello world", 6, "<...>d"}, + {"hello world", 5, "<...>"}, + {"have := truncateText(test.input, test.maxLen)", 20, "have :=<...>.maxLen)"}, + {"have := truncateText(test.input, test.maxLen)", 30, "have := trun<...> test.maxLen)"}, + {"have := truncateText(test.input, test.maxLen)", 40, "have := truncateT<...>nput, test.maxLen)"}, + {"have := truncateText(test.input, test.maxLen)", 41, "have := truncateTe<...>nput, test.maxLen)"}, + {"have := truncateText(test.input, test.maxLen)", 42, "have := truncateTe<...>input, test.maxLen)"}, + {"have := truncateText(test.input, test.maxLen)", 50, "have := truncateText(test.input, test.maxLen)"}, + } + + for _, test := range tests { + have := truncateText(test.input, test.maxLen) + if len(have) > test.maxLen { + t.Errorf("truncateText(%q, %v): len %d exceeeds max len", + test.input, test.maxLen, len(have)) + } + if len(test.input) > test.maxLen && len(have) != test.maxLen { + t.Errorf("truncateText(%q, %v): truncated more than necessary", + test.input, test.maxLen) + } + if have != test.want { + t.Fatalf("truncateText(%q, %v):\nhave: %q\nwant: %q", + test.input, test.maxLen, have, test.want) + } + } +} + func TestRenderMessage(t *testing.T) { tests := []struct { msg string diff --git a/ruleguard/runner.go b/ruleguard/runner.go index 8b7824db..3c690754 100644 --- a/ruleguard/runner.go +++ b/ruleguard/runner.go @@ -375,14 +375,24 @@ func (rr *rulesRunner) renderMessage(msg string, m matchData, truncate bool) str } buf.Reset() buf.Write(rr.nodeText(n)) - // Don't interpolate strings that are too long. - var replacement string - if truncate && buf.Len() > 60 { - replacement = key - } else { - replacement = buf.String() + replacement := buf.String() + if truncate { + replacement = truncateText(replacement, 60) } msg = strings.ReplaceAll(msg, key, replacement) } return msg } + +func truncateText(s string, maxLen int) string { + const placeholder = "<...>" + if len(s) <= maxLen-len(placeholder) { + return s + } + maxLen -= len(placeholder) + leftLen := maxLen / 2 + rightLen := (maxLen % 2) + leftLen + left := s[:leftLen] + right := s[len(s)-rightLen:] + return left + placeholder + right +}