func (s *baseSuite) TestSkipContextSkipsIssue(c *gc.C) {
b := s.Tenet.(*tenet.Base)
// no regisitered issues should mean there is no context, which should
// mean all contexts have been matched ...
s.assertCxtFull(c, true)
// ... and thus adding a new comment with a new context means we have a
// context that hasn't matched.
b.RegisterIssue("issue_with_first_comment_ctx",
tenet.AddComment("first comment", tenet.FirstComment),
tenet.AddComment("third comment", tenet.ThirdComment),
)
s.assertCxtFull(c, false)
// Add a smell which raises the above issue for every line.
b.SmellLine(func(r tenet.Review, n int, line []byte) error {
r.RaiseLineIssue("issue_with_first_comment_ctx", n, n)
return nil
})
// Review some code, raise the issue.
s.CheckSRC(c, "package mock\n// 2nd line\n// 3rd line", []tt.ExpectedIssue{
{
Text: "package mock",
Comment: "first comment",
}, {
Text: "// 3rd line",
Comment: "third comment",
}}...)
s.assertCxtFull(c, true)
}
func New() *slasherTenet {
t := &slasherTenet{}
t.SetInfo(tenet.Info{
Name: "slasher",
Usage: "Comments should have a space after '//' forward slashes",
Description: "Comments should have a space after '//' forward slashes",
SearchTags: []string{"format", "comment", "doc-comment"},
Language: "golang",
})
confidence := t.RegisterMetric("confidence")
issue := t.RegisterIssue("no_space_after_comment",
tenet.AddComment("You need a space after the '//'", tenet.FirstComment),
tenet.AddComment("Here needs a space also.", tenet.SecondComment),
tenet.AddComment("And so on, please always have a space.", tenet.ThirdComment),
)
t.SmellLine(func(r tenet.Review, n int, line []byte) error {
if regexp.MustCompile(`\/\/[^\s]{1}`).Match(line) {
r.RaiseLineIssue(issue, n, n, confidence(0.9))
}
return nil
})
return t
}
func New() *unusedArgTenet {
t := &unusedArgTenet{}
t.SetInfo(tenet.Info{
Name: "unused_arg",
Usage: "catch funcs that don't use an argument",
Description: "Ensure a function's arguments are used in the body of the function.",
SearchTags: []string{"function", "method"},
Language: "golang",
})
confidence := t.RegisterMetric("confidence")
issue := t.RegisterIssue("unused_func_arg",
// TODO(waigani) "Argument{s} {args} {is_are} not used in the function's body."
tenet.AddComment(`{{.args}} used. If this is intentional, please change the name to "_".`, tenet.FirstComment),
tenet.AddComment(`{{.args}} used. Please remove or set the name to "_".`, tenet.SecondComment),
tenet.AddComment(`{{.args}} used. Set to "_" or remove.`, tenet.ThirdComment),
tenet.AddComment(`{{.args}} used. You get the idea ...`, tenet.FourthComment),
)
t.SmellNode(func(r tenet.Review, fnc *ast.FuncDecl) error {
args := fnc.Type.Params.List
if len(args) == 0 {
return nil
}
v := &visitor{
args: map[string]bool{},
}
for _, arg := range args {
for _, ident := range arg.Names {
n := ident.Name
if n != "" && n != "_" {
v.args[n] = true
}
}
}
ast.Walk(v, fnc.Body)
if len(v.args) > 0 {
names := make([]string, 0, len(v.args))
for k := range v.args {
names = append(names, k)
}
unused := `"` + strings.Join(names, `", "`)
if len(v.args) > 1 {
unused += `" aren't`
} else {
unused += `" isn't`
}
r.RaiseNodeIssue(issue, fnc, confidence(0.5), tenet.CommentVar("args", unused))
}
return nil
})
return t
}
func New() *licenseTenet {
t := &licenseTenet{}
t.SetInfo(tenet.Info{
Name: "license",
Usage: "Each file should contain the appropriate license header.",
Description: "Ensure that each file in the project begins with a license: \"{{.header}}\"",
SearchTags: []string{"license", "comment", "doc-comment"},
Language: "go",
})
confidence := t.RegisterMetric("confidence")
issue := t.RegisterIssue("incorrect_header", tenet.AddComment(`
Each file should start with a license header of the following format:
{{.header}}
`[1:], tenet.FirstComment),
tenet.AddComment("This file also needs the correctly formatted license header.", tenet.SecondComment),
tenet.AddComment("And so on, license header again.", tenet.ThirdComment),
)
// RegisterOption returns a pointer to a string. The string will be
// updated by the time it is used in the smell.
headerPnt := t.RegisterOption("header", "Copyright Me", "the license header to check for")
t.SmellLine(func(r tenet.Review, n int, line []byte) error {
headerVal := *headerPnt
lineMatchesHeader := func() bool {
var i int
header := strings.Split(headerVal, "\n")
for i = 1; i <= len(header); i++ {
if n == i && string(line) != header[i-1] {
// It doesn't match, no need to keep sniffing this file.
r.SmellDoneWithFile()
return false
}
}
if n == i {
fmt.Println(n, i, string(line), headerVal)
// Every line of the header matches. Don't check again.
r.SmellDoneWithFile()
}
return true
}
if !lineMatchesHeader() {
r.RaiseLineIssue(issue, 1, 1, confidence(0.7), tenet.CommentVar("header", headerVal))
}
return nil
})
return t
}
func New() *importsTenet {
t := &importsTenet{}
t.SetInfo(tenet.Info{
Name: "imports",
Usage: "police the imports of a package",
Description: "imports matching the following regex are restricted: \"{{.blacklist_regex}}\"",
SearchTags: []string{"import"},
Language: "go",
})
blacklisted := t.RegisterOption("blacklist_regex", "", "a regex to filter imports against")
issue := t.RegisterIssue("blacklisted_import",
tenet.AddComment(`This package should not be bringing in {{.importName}}`),
)
// Metrics and tags could also be registered at this point to help track
// this type of issue.
t.SmellNode(func(r tenet.Review, imp *ast.ImportSpec) error {
importName := imp.Path.Value
m, err := regexp.MatchString(*blacklisted, importName)
if err != nil {
return errors.Trace(err)
}
if m {
r.RaiseNodeIssue(issue, imp, tenet.CommentVar("importName", importName))
}
return nil
})
return t
}
func (s *baseSuite) TestDefaultFindsEveryIssue(c *gc.C) {
b := s.Tenet.(*tenet.Base)
// Adding an issue with no comment context defaults to hardcoded default
// ...
b.RegisterIssue("issue_with_every_line", tenet.AddComment("Custom Default Msg", tenet.DefaultComment))
// Add a smell which raises the above issue for every line.
b.SmellLine(func(r tenet.Review, n int, line []byte) error {
r.RaiseLineIssue("issue_with_every_line", n, n)
return nil
})
// ... and picks up every issue.
s.CheckSRC(c, "package mock\n// 2nd line\n// 3rd line", []tt.ExpectedIssue{
{
Text: "package mock",
Comment: "Custom Default Msg",
}, {
Text: "// 2nd line",
Comment: "Custom Default Msg",
}, {
Text: "// 3rd line",
Comment: "Custom Default Msg",
}}...)
s.assertCxtFull(c, false)
}
func New() *noStateTenet {
t := &noStateTenet{}
t.SetInfo(tenet.Info{
Name: "worker_nostate",
Usage: "workers should not access state directly",
Description: "If you're passing a \\*state.State into your worker, you are almost certainly doing it wrong. The layers go worker->apiserver->state, and any attempt to skip past the apiserver layer should be viewed with *extreme* suspicion.",
Language: "golang",
SearchTags: []string{"juju", "worker"},
})
// We register any issues, metrics and tags that we'll be using.
t.confidence = t.RegisterMetric("confidence")
t.observability = t.RegisterTag("observability")
t.maybeIssue = t.RegisterIssue("imports_state_returns_worker",
tenet.AddComment(`
I see you've imported state. A worker shouldn't need it. Best practice for writing workers:
https://github.com/juju/juju/wiki/Guidelines-for-writing-workers
`[1:]),
)
t.mostLikelyIssue = t.RegisterIssue("func_takes_state_returns_worker",
tenet.AddComment(`
Please don't pass in a state object here. Workers should use the API.
More info here: https://github.com/juju/juju/wiki/Guidelines-for-writing-workers
`[1:]),
)
// First, let's knock out any file that doesn't import state and worker.
t.SmellNode(func(r tenet.Review, astFile *ast.File) error {
if !astutil.UsesImport(astFile, "github.com/juju/juju/state") ||
!astutil.UsesImport(astFile, "github.com/juju/juju/worker") {
// This file will no longer be smelt by this tenet.
r.FileDone()
}
return nil
})
t.smellFuncs()
return t
}
func (s *baseSuite) TestSkipFallsBackToCustomDefaultFindsEveryIssue(c *gc.C) {
b := s.Tenet.(*tenet.Base)
// Adding an issue with no comment context defaults to hardcoded default
// ...
b.RegisterIssue("issue_with_every_line",
tenet.AddComment("first comment", tenet.FirstComment),
tenet.AddComment("third comment", tenet.ThirdComment),
tenet.AddComment("Custom Default Msg"),
)
// Add a smell which raises the above issue for every line.
b.SmellLine(func(r tenet.Review, n int, line []byte) error {
r.RaiseLineIssue("issue_with_every_line", n, n)
return nil
})
// ... and picks up every issue.
s.CheckSRC(c, `
package mock
// 2nd line
// 3rd line
// 4th line
`[1:], []tt.ExpectedIssue{
{
Text: "package mock",
Comment: "first comment",
}, {
Text: "// 2nd line",
Comment: "Custom Default Msg",
}, {
Text: "// 3rd line",
Comment: "third comment",
}, {
Text: "// 4th line",
Comment: "Custom Default Msg",
}, {
Text: "", // new line at end of file
Comment: "Custom Default Msg",
}}...)
s.assertCxtFull(c, false)
}
// TODO(waigani) table based test of all context combinations.
func (s *baseSuite) TestContextFirstInEveryFile(c *gc.C) {
b := s.Tenet.(*tenet.Base)
b.RegisterIssue("issue_with_every_line",
tenet.AddComment("first comment", tenet.FirstComment, tenet.InEveryFile),
)
// Add a smell which raises the above issue for every line.
b.SmellLine(func(r tenet.Review, n int, line []byte) error {
r.RaiseLineIssue("issue_with_every_line", n, n)
return nil
})
// Add three files with a line of code.
src := `
package mock
// 2nd line
// 3rd line
// 4th line
`[1:]
files := []string{
s.TmpFile(c, src),
s.TmpFile(c, src),
s.TmpFile(c, src),
}
expectedIssues := []tt.ExpectedIssue{
{
Text: "package mock",
Comment: "first comment",
Filename: files[0],
}, {
Text: "package mock",
Comment: "first comment",
Filename: files[1],
}, {
Text: "package mock",
Comment: "first comment",
Filename: files[2],
},
}
s.CheckFiles(c, files, expectedIssues...)
// Assert the context is now matched.
s.assertCxtFull(c, false)
}
// a comment
func main() {
c := &commentTenet{}
c.SetInfo(tenet.Info{
Name: "fif_node",
Usage: "take issue with the first comment in every file",
Language: "Go",
Description: `
This tenet is part of lingo's internal testing suite. It should raise an issue for the
first comment encountered in every file, using SmellNode.
`,
})
issue := c.RegisterIssue("comment", tenet.AddComment("first comment - node", tenet.FirstComment, tenet.InEveryFile))
c.SmellNode(func(r tenet.Review, comment *ast.Comment) error {
r.RaiseNodeIssue(issue, comment)
return nil
})
server.Serve(c)
}
// a comment
func main() {
c := &commentTenet{}
c.SetInfo(tenet.Info{
Name: "fif_line",
Usage: "take issue with the first comment in every file",
Language: "Go",
Description: `
This tenet is part of lingo's internal testing suite. It should raise an issue for the
first comment encountered in every file, using SmellLine.
`,
})
issue := c.RegisterIssue("comment", tenet.AddComment("first comment - line", tenet.FirstComment, tenet.InEveryFile))
c.SmellLine(func(r tenet.Review, n int, line []byte) error {
if regexp.MustCompile(`\/\/.*`).Match(line) {
r.RaiseLineIssue(issue, n, n)
}
return nil
})
server.Serve(c)
}
// a comment
func main() {
c := &commentTenet{}
c.SetInfo(tenet.Info{
Name: "simpleseed",
Usage: "every comment should be awesome",
Language: "Go",
Description: `
simpleseed is a demonstration tenet showing the structure required
to write a tenet in Go. When reviewing code with simpleseed it will be suggested
that all comments could be more awesome.
`,
})
issue := c.RegisterIssue("sucky_comment", tenet.AddComment("this comment could be more awesome"))
c.SmellNode(func(r tenet.Review, comment *ast.Comment) error {
if comment.Text != "// most awesome comment ever" {
r.RaiseNodeIssue(issue, comment)
}
return nil
})
server.Serve(c)
}
// TODO(waigani) table based test of all context combinations.
func (s *baseSuite) TestContextDefaultInSecondFile(c *gc.C) {
b := s.Tenet.(*tenet.Base)
b.RegisterIssue("issue_with_every_line",
tenet.AddComment("first comment", tenet.DefaultComment, tenet.InSecondFile),
)
// Add a smell which raises the above issue for every line.
b.SmellLine(func(r tenet.Review, n int, line []byte) error {
r.RaiseLineIssue("issue_with_every_line", n, n)
return nil
})
// Add three files with a line of code.
src := `
package mock
// 2nd line
// 3rd line
// 4th line
`[1:]
src2 := `
package secondMock
// 2.2nd line
// 2.3rd line
// 2.4th line
`[1:]
files := []string{
s.TmpFile(c, src),
s.TmpFile(c, src2),
s.TmpFile(c, src),
}
expectedIssues := []tt.ExpectedIssue{
{
Text: "package secondMock",
Comment: "first comment",
Filename: files[1],
}, {
Text: "// 2.2nd line",
Comment: "first comment",
Filename: files[1],
}, {
Text: "// 2.3rd line",
Comment: "first comment",
Filename: files[1],
}, {
Text: "// 2.4th line",
Comment: "first comment",
Filename: files[1],
}, {
Text: "", // new line at end of file
Comment: "first comment",
},
}
s.CheckFiles(c, files, expectedIssues...)
// TODO(waigani) this should be true. Once we sniff the last node/line of
// a file, we can set DefaultComment|FileContext as matched
s.assertCxtFull(c, false)
}
func New() *assertLoopLenTenet {
t := &assertLoopLenTenet{}
t.SetInfo(tenet.Info{
Name: "juju_test_assert_loop_len",
Usage: "If asserting within a loop, the length of the colleciton being iterated should be asserted",
Description: "If asserting within a loop, the length of the colleciton being iterated should be asserted",
SearchTags: []string{"test", "loop"},
Language: "go",
})
assertLoopIssue := t.RegisterIssue("loop_len_not_asserted",
tenet.AddComment(`The asserts within this loop may never get run. The length of "{{.looped}}" needs to be asserted.`, tenet.FirstComment),
tenet.AddComment(`The length of "{{.looped}}" needs to be asserted also`, tenet.DefaultComment),
)
// TODO(waigani) good canditate for a patch fix.
rangeCallExpIssue := t.RegisterIssue("range_over_call_exp",
tenet.AddComment(`
Even if you assert the length of the result of this call before iterating
over it, you cannot guarantee the result will be the same each time you call
it. You cannot be sure that the asserts within the for loop will get
run. Please assign the result of {{.looped}} to a variable, assert the expected
length of the variable and then loop over that.`[1:], tenet.FirstComment),
tenet.AddComment(`
Here also, you can't gurantee the length of {{.looped}}'s result.`[1:], tenet.SecondComment),
tenet.AddComment(`
Again, need to assert result of {{.looped}} first.`[1:], tenet.DefaultComment),
)
// // First, knock out any file that isn't a test
t.SmellNode(func(r tenet.Review, _ *ast.File) error {
if !strings.HasSuffix(r.File().Filename(), "_test.go") {
r.FileDone()
}
return nil
})
// All nodes that have been asserted in a loop.
var ranged possibleBadRange
// Check if any range body contains an assert or check.
t.SmellNode(func(r tenet.Review, rngStmt *ast.RangeStmt) error {
// TODO(waigani) need to return inc statements and check they are
// asserted after loop.
if containsIncStmt(rngStmt.Body.List) {
return nil
}
if containsCheckOrAssert(rngStmt.Body.List) {
switch n := rngStmt.X.(type) {
case *ast.CallExpr:
looped := "the call"
switch x := n.Fun.(type) {
case *ast.Ident:
looped = x.Name + "()"
case *ast.SelectorExpr:
looped = x.Sel.Name + "()"
}
r.RaiseNodeIssue(rangeCallExpIssue, n, tenet.CommentVar("looped", looped))
case *ast.Ident:
ranged.add(n)
default:
// TODO(waigani) log unknown range symbol
}
}
return nil
})
// Find any idents that have been constructed in this file.
t.SmellNode(func(r tenet.Review, ident *ast.Ident) error {
if ranged.empty() {
// Nothing was found to be ranged over. No need to keep smelling.
r.FileDone()
}
// This ident has not been ranged over, so we're not interested in it.
if !ranged.has(ident) {
return nil
}
isCompLit, err := astutil.DeclaredWithLit(ident)
if err != nil {
return errors.Trace(err)
}
if isCompLit {
// The var has been constructed in this file, so it is clear
// to see it's length. We are no longer interested in it.
ranged.remove(ident)
}
return nil
})
// Check that any remaining ranged vars not constructed in this file have had their lengths asserted.
t.SmellNode(func(r tenet.Review, callExpr *ast.CallExpr) error {
if fun, ok := callExpr.Fun.(*ast.SelectorExpr); ok {
if fun.Sel.String() != "Assert" && fun.Sel.String() != "Check" {
return nil
}
if args := callExpr.Args; len(args) == 3 {
switch n := args[0].(type) {
case *ast.Ident:
// we have an assert on the collection.
if sel, ok := args[1].(*ast.SelectorExpr); ok {
if sel.Sel.String() == "HasLen" {
// The length has been asserted, wer're no longer
// interested in this var. Remove it if we added it.
ranged.remove(n)
return nil
}
}
case *ast.CallExpr:
if f, ok := n.Fun.(*ast.Ident); ok {
if f.Name == "len" {
if x, ok := n.Args[0].(*ast.Ident); ok {
// The length has been asserted, wer're no longer
// interested in this var. Remove it if we added it.
ranged.remove(x)
}
}
}
}
}
}
return nil
})
// Do one more run over range statements.
t.SmellNode(func(r tenet.Review, rngStmt *ast.RangeStmt) error {
if ranged.empty() {
// There are no possible vars of unasserted len in this file.
// Don't run any more smells.
r.FileDone()
}
// Find our assert ranges again
if containsCheckOrAssert(rngStmt.Body.List) {
switch n := rngStmt.X.(type) {
case *ast.Ident:
// we're only interested in idents this time
if ranged.has(n) {
r.RaiseNodeIssue(assertLoopIssue, n, tenet.CommentVar("looped", n.Name))
}
}
}
return nil
})
return t
}
func New() *unusedArgTenet {
t := &unusedArgTenet{}
t.SetInfo(tenet.Info{
Name: "exhaustive_switch",
Usage: "ensure that a switch has a case for every possible value of a variable",
Description: "Ensure that a switch has a case for every possible value of a variable",
SearchTags: []string{"switch"},
Language: "golang",
})
exhaustTag := t.RegisterOption("exhaust_tag", "lingo:exhaustive", "If this tag is found in a comment above a switch statement, that switch will be treated as an exhaustive switch.")
issue := t.RegisterIssue("missing_case",
tenet.AddComment(`The following cases are missing from this switch:{{.cases}}.`),
)
// map of a switched variable to all case values.
switchedVarSets := map[int]map[string][]ast.Expr{}
t.SmellNode(func(r tenet.Review, file *ast.File) error {
for _, com := range file.Comments {
if strings.Contains(com.Text(), *exhaustTag) {
switchedVarSets[r.File().Fset().Position(com.End()).Line+1] = map[string][]ast.Expr{}
}
}
return nil
})
// First find switched vars
t.SmellNode(func(r tenet.Review, swt *ast.SwitchStmt) error {
// Did we find a tag for this switch?
swtLine := r.File().Fset().Position(swt.Pos()).Line
if _, ok := switchedVarSets[swtLine]; !ok {
return nil
}
var switchedVar string
if ident, ok := swt.Tag.(*ast.Ident); ok {
var err error
switchedVar, err = astutil.TypeOf(ident)
if err != nil {
return errors.Trace(err)
}
} else {
return errors.Errorf("found switch, but could not get variable name: %#v", swt.Tag)
}
for _, stm := range swt.Body.List {
if c, ok := stm.(*ast.CaseClause); ok {
for _, l := range c.List {
switchedVarSets[swtLine][switchedVar] = append(switchedVarSets[swtLine][switchedVar], l)
}
}
}
return nil
})
missingCases := map[int][]*ast.Ident{}
// Then find genDecls with the switched type in it and make sure all
// GenDecls of that type are switched on.
t.SmellNode(func(r tenet.Review, genDec *ast.GenDecl) error {
if len(switchedVarSets) == 0 {
r.FileDone()
}
for switchLine, switchSet := range switchedVarSets {
for switchType := range switchSet {
var inGenDecl bool
// first see if a switched type is in this genDecl
for _, s := range genDec.Specs {
if valSpec, ok := s.(*ast.ValueSpec); ok {
for _, n := range valSpec.Names {
typ, err := astutil.TypeOf(n)
if err != nil {
// TODO(waigani) log
continue
}
// Does the type in this GenDecl match that of the
// switch?
if typ == switchType {
inGenDecl = true
}
}
}
}
if !inGenDecl {
continue
}
// Now make sure there is a case for each GenGecl
for _, s := range genDec.Specs {
if valSpec, ok := s.(*ast.ValueSpec); ok {
for _, vName := range valSpec.Names {
// Does our switch contain the var from genDecl?
if !contains(vName, switchedVarSets[switchLine][switchType]) {
missingCases[switchLine] = append(missingCases[switchLine], vName)
}
}
}
}
}
}
return nil
})
// Do another sweep over switches, this time raising an issue for any in the missingCases map
t.SmellNode(func(r tenet.Review, swt *ast.SwitchStmt) error {
if len(missingCases) == 0 {
// no switches with missing cases were found. Don't smell any more
// nodes.
r.SmellDone()
return nil
}
switchLine := r.File().Fset().Position(swt.Pos()).Line
if cases, ok := missingCases[switchLine]; ok {
var missing string
for _, m := range cases {
missing += ", " + m.String()
}
r.RaiseNodeIssue(issue, swt, tenet.CommentVar("cases", missing[1:]))
}
return nil
})
return t
}
