From d5686d03c0a55a6a2e2fedc7d69515b07c328664 Mon Sep 17 00:00:00 2001 From: ashmrtn <3891298+ashmrtn@users.noreply.github.com> Date: Mon, 19 Jun 2023 09:12:55 -0700 Subject: [PATCH 1/2] Flip test comment flag name and value Provides a better UX as specifying a falsy value for a flag can be a bit annoying on CLI. --- pkg/commentmimic/commentmimic.go | 12 ++++++------ pkg/commentmimic/commentmimic_test.go | 2 +- testdata/script/no_test_comments.txtar | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/commentmimic/commentmimic.go b/pkg/commentmimic/commentmimic.go index 13738fb..95bf0d2 100644 --- a/pkg/commentmimic/commentmimic.go +++ b/pkg/commentmimic/commentmimic.go @@ -365,7 +365,7 @@ const ( CommentExportedFuncsFlag = "comment-exported" CommentAllExportedFuncsFlag = "comment-all-exported" CommentInterfacesFlag = "comment-interfaces" - NoTestCommentsFlag = "no-test-comments" + CommentTestsFlag = "comment-tests" CommentStructsFlag = "comment-structs" ) @@ -374,7 +374,7 @@ type mimic struct { commentAllExportedFuncs bool commentInterfaces bool commentStructs bool - noTestComments bool + commentTests bool } func New() *analysis.Analyzer { @@ -403,10 +403,10 @@ func New() *analysis.Analyzer { ) fs.BoolVar( - &m.noTestComments, - NoTestCommentsFlag, - true, - "don't require comments on tests, benchmarks, examples, and fuzz tests", + &m.commentTests, + CommentTestsFlag, + false, + "require comments on tests, benchmarks, examples, and fuzz tests", ) fs.BoolVar( diff --git a/pkg/commentmimic/commentmimic_test.go b/pkg/commentmimic/commentmimic_test.go index 45989a2..633688e 100644 --- a/pkg/commentmimic/commentmimic_test.go +++ b/pkg/commentmimic/commentmimic_test.go @@ -752,7 +752,7 @@ func (s *CommentMimicSuite) TestSkipTestComments() { commentmimic.CommentAllExportedFuncsFlag: true, commentmimic.CommentInterfacesFlag: true, commentmimic.CommentStructsFlag: true, - commentmimic.NoTestCommentsFlag: true, + commentmimic.CommentTestsFlag: false, } fileMap := map[string]string{ diff --git a/testdata/script/no_test_comments.txtar b/testdata/script/no_test_comments.txtar index 88b1632..b776330 100644 --- a/testdata/script/no_test_comments.txtar +++ b/testdata/script/no_test_comments.txtar @@ -1,4 +1,4 @@ -! exec commentmimic --comment-all-exported --no-test-comments not_commented_errors.go not_commented_errors_test.go +! exec commentmimic --comment-all-exported not_commented_errors.go not_commented_errors_test.go stderr -count=4 'exported element ''Func[A-D]'' should be commented' -- not_commented_errors.go -- From e727f32f77f00825dc677eb683f87d3478f867ec Mon Sep 17 00:00:00 2001 From: ashmrtn <3891298+ashmrtn@users.noreply.github.com> Date: Mon, 19 Jun 2023 09:13:53 -0700 Subject: [PATCH 2/2] Fix bug in logic for ignoring test files Only ignore test files if we're not checking them instead of ignoring them all the time. Add tests to make sure this doesn't regress in the future. --- pkg/commentmimic/commentmimic.go | 2 +- pkg/commentmimic/commentmimic_test.go | 56 +++++++++++++++++++++++ pkg/commentmimic/testdata/out_of_scope.go | 27 +++++++++++ 3 files changed, 84 insertions(+), 1 deletion(-) diff --git a/pkg/commentmimic/commentmimic.go b/pkg/commentmimic/commentmimic.go index 95bf0d2..ccd4f32 100644 --- a/pkg/commentmimic/commentmimic.go +++ b/pkg/commentmimic/commentmimic.go @@ -236,7 +236,7 @@ func (m mimic) checkFuncDecl(pass *analysis.Pass, fun *ast.FuncDecl) { commentExported := m.commentExportedFuncs commentAllExported := m.commentAllExportedFuncs - if isTestFunc(pass.Fset, fun.Pos(), fun.Name.Name) { + if !m.commentTests && isTestFunc(pass.Fset, fun.Pos(), fun.Name.Name) { commentExported = false commentAllExported = false } diff --git a/pkg/commentmimic/commentmimic_test.go b/pkg/commentmimic/commentmimic_test.go index 633688e..1817b73 100644 --- a/pkg/commentmimic/commentmimic_test.go +++ b/pkg/commentmimic/commentmimic_test.go @@ -773,6 +773,62 @@ func (s *CommentMimicSuite) TestSkipTestComments() { analysistest.Run(t, dir, mimic, "./...") } +func (s *CommentMimicSuite) TestTestComments() { + table := []struct { + name string + input string + flags map[string]bool + }{ + { + name: "DontRequireComments", + input: testdata.SkipTestComments, + flags: map[string]bool{ + commentmimic.CommentExportedFuncsFlag: true, + commentmimic.CommentAllExportedFuncsFlag: true, + commentmimic.CommentInterfacesFlag: true, + commentmimic.CommentStructsFlag: true, + commentmimic.CommentTestsFlag: false, + }, + }, + { + name: "RequireComments", + input: testdata.TestComments, + flags: map[string]bool{ + commentmimic.CommentExportedFuncsFlag: true, + commentmimic.CommentAllExportedFuncsFlag: true, + commentmimic.CommentInterfacesFlag: true, + commentmimic.CommentStructsFlag: true, + commentmimic.CommentTestsFlag: true, + }, + }, + } + + for _, test := range table { + test := test + + s.T().Run(test.name, func(t *testing.T) { + t.Parallel() + + fileMap := map[string]string{ + "a/a_test.go": test.input, + } + + dir, cleanup, err := analysistest.WriteFiles(fileMap) + require.NoError(t, err) + + defer cleanup() + + mimic := commentmimic.New() + + for flag, value := range test.flags { + require.NoError(t, mimic.Flags.Set(flag, strconv.FormatBool(value))) + } + + analysistest.Run(t, dir, mimic, "./...") + }) + } +} + func (s *CommentMimicSuite) TestFuncCommentErrors() { element := "element" base := generateCommentMimicCases(element) diff --git a/pkg/commentmimic/testdata/out_of_scope.go b/pkg/commentmimic/testdata/out_of_scope.go index 87a340e..6b5162b 100644 --- a/pkg/commentmimic/testdata/out_of_scope.go +++ b/pkg/commentmimic/testdata/out_of_scope.go @@ -197,4 +197,31 @@ type testIface interface { func TestDoesntNeedComment(t *testing.T) {} ` + + TestComments = `package a_test + + import ( + "testing" + ) + + type BenchmarkInterface interface {} // want "exported element 'BenchmarkInterface' should be commented" + + type ExampleInterface interface {} // want "exported element 'ExampleInterface' should be commented" + + type FuzzInterface interface {} // want "exported element 'FuzzInterface' should be commented" + + type TestInterface interface {} // want "exported element 'TestInterface' should be commented" + + func BenchmarkNeedsComment(b *testing.B) {} // want "exported element 'BenchmarkNeedsComment' should be commented" + + func ExportedElement() bool { // want "exported element 'ExportedElement' should be commented" + return false + } + + func ExampleNeedsComment() {} // want "exported element 'ExampleNeedsComment' should be commented" + + func FuzzNeedsComment(f *testing.F) {} // want "exported element 'FuzzNeedsComment' should be commented" + + func TestNeedsComment(t *testing.T) {} // want "exported element 'TestNeedsComment' should be commented" +` )