diff --git a/checks/AZC/AZC001_client_missing_resource_manager_endpoint/AZC001.go b/checks/AZC/AZC001_client_missing_resource_manager_endpoint/AZC001.go index 8d5a38b..1bc823e 100644 --- a/checks/AZC/AZC001_client_missing_resource_manager_endpoint/AZC001.go +++ b/checks/AZC/AZC001_client_missing_resource_manager_endpoint/AZC001.go @@ -52,8 +52,7 @@ func run(pass *analysis.Pass) (any, error) { return } - pass.Reportf(call.Pos(), - "Azure SDK clients should be created with NewFoosClientWithBaseURI and the resource manager endpoint explicitly specified") + pass.Reportf(call.Pos(), "Azure SDK clients should be created with NewFoosClientWithBaseURI and the resource manager endpoint explicitly specified") }) return nil, nil diff --git a/checks/AZD/AZD001_data_source_empty_set_id/AZD001.go b/checks/AZD/AZD001_data_source_empty_set_id/AZD001.go index fceee99..0c85efb 100644 --- a/checks/AZD/AZD001_data_source_empty_set_id/AZD001.go +++ b/checks/AZD/AZD001_data_source_empty_set_id/AZD001.go @@ -52,8 +52,7 @@ func run(pass *analysis.Pass) (any, error) { return } - pass.Reportf(call.Pos(), - "data sources should return an error when a resource cannot be found instead of calling SetId with an empty string") + pass.Reportf(call.Pos(), "data sources should return an error when a resource cannot be found instead of calling SetId with an empty string") }) return nil, nil diff --git a/checks/AZD/AZD002_data_source_mark_as_gone/AZD002.go b/checks/AZD/AZD002_data_source_mark_as_gone/AZD002.go index 65cf1f7..f009941 100644 --- a/checks/AZD/AZD002_data_source_mark_as_gone/AZD002.go +++ b/checks/AZD/AZD002_data_source_mark_as_gone/AZD002.go @@ -41,8 +41,7 @@ func run(pass *analysis.Pass) (any, error) { return } - pass.Reportf(sel.Pos(), - "data sources should return an error when a resource cannot be found instead of calling MarkAsGone") + pass.Reportf(sel.Pos(), "data sources should return an error when a resource cannot be found instead of calling MarkAsGone") }) return nil, nil diff --git a/checks/AZG/AZG000_azignore_missing_reason/AZG000.go b/checks/AZG/AZG000_azignore_missing_reason/AZG000.go index 89f3ee3..dc89009 100644 --- a/checks/AZG/AZG000_azignore_missing_reason/AZG000.go +++ b/checks/AZG/AZG000_azignore_missing_reason/AZG000.go @@ -26,8 +26,7 @@ func run(pass *analysis.Pass) (any, error) { continue } - pass.Reportf(comment.Pos(), - "azignore directive must include a reason: '//azignore: - '") + pass.Reportf(comment.Pos(), "azignore directive must include a reason: '//azignore: - '") } } } diff --git a/checks/AZG/AZG001_combine_err_assignment_and_check/AZG001.go b/checks/AZG/AZG001_combine_err_assignment_and_check/AZG001.go index 116de2f..32c554b 100644 --- a/checks/AZG/AZG001_combine_err_assignment_and_check/AZG001.go +++ b/checks/AZG/AZG001_combine_err_assignment_and_check/AZG001.go @@ -68,12 +68,14 @@ func checkBlock(pass *analysis.Pass, stmts []ast.Stmt) { lhsIdents = append(lhsIdents, ident) lhsNames = append(lhsNames, ident.Name) } + if len(lhsNames) != len(assignStmt.Lhs) { continue } if lhsNames[len(lhsNames)-1] != "err" { continue } + allBlankPrefix := true for _, name := range lhsNames[:len(lhsNames)-1] { if name != "_" { @@ -81,9 +83,11 @@ func checkBlock(pass *analysis.Pass, stmts []ast.Stmt) { break } } + if !allBlankPrefix { continue } + errIdent := lhsIdents[len(lhsIdents)-1] // Next statement must be `if err != nil` @@ -123,10 +127,8 @@ func checkBlock(pass *analysis.Pass, stmts []ast.Stmt) { } pass.Report(analysis.Diagnostic{ - Pos: assignStmt.Pos(), - Message: fmt.Sprintf( - "'%s' assignment should be combined with the following 'if err != nil' into a single 'if' init statement", - strings.Join(lhsNames, ", ")), + Pos: assignStmt.Pos(), + Message: fmt.Sprintf("'%s' assignment should be combined with the following 'if err != nil' into a single 'if' init statement", strings.Join(lhsNames, ", ")), SuggestedFixes: fixes, }) } diff --git a/checks/AZG/AZG002_address_of_single_use_temporary/AZG002.go b/checks/AZG/AZG002_address_of_single_use_temporary/AZG002.go index 006f210..b09801f 100644 --- a/checks/AZG/AZG002_address_of_single_use_temporary/AZG002.go +++ b/checks/AZG/AZG002_address_of_single_use_temporary/AZG002.go @@ -75,14 +75,10 @@ var fixPointerCopy string func init() { Analyzer.Flags.Init("AZG002", flag.ContinueOnError) - Analyzer.Flags.IntVar(&maxGap, "max-gap", 100, - "maximum number of source lines between the temporary's declaration and the statement taking its address") - Analyzer.Flags.StringVar(&use, "use", useNew, - "pointer-creation form to suggest: new or pointer.To (packages below go1.26 must set pointer.To)") - Analyzer.Flags.StringVar(&allow, "allow", "", - "comma-separated forms to leave unreported where they already appear: pointer.To") - Analyzer.Flags.StringVar(&fixPointerCopy, "fix-pointer-copy", fixPointerCopyNone, - "fix for a dereference-copy temporary (out := *p; &out): none, copy, or share") + Analyzer.Flags.IntVar(&maxGap, "max-gap", 100, "maximum number of source lines between the temporary's declaration and the statement taking its address") + Analyzer.Flags.StringVar(&use, "use", useNew, "pointer-creation form to suggest: new or pointer.To (packages below go1.26 must set pointer.To)") + Analyzer.Flags.StringVar(&allow, "allow", "", "comma-separated forms to leave unreported where they already appear: pointer.To") + Analyzer.Flags.StringVar(&fixPointerCopy, "fix-pointer-copy", fixPointerCopyNone, "fix for a dereference-copy temporary (out := *p; &out): none, copy, or share") } func run(pass *analysis.Pass) (any, error) { @@ -97,6 +93,7 @@ func run(pass *analysis.Pass) (any, error) { if fixPointerCopy != fixPointerCopyNone && fixPointerCopy != fixPointerCopyCopy && fixPointerCopy != fixPointerCopyShare { return nil, fmt.Errorf("AZG002: invalid fix-pointer-copy flag %q: expected %q, %q or %q", fixPointerCopy, fixPointerCopyNone, fixPointerCopyCopy, fixPointerCopyShare) } + allowed := map[string]bool{} for form := range strings.SplitSeq(allow, ",") { form = strings.TrimSpace(form) @@ -230,11 +227,12 @@ func checkPair(pass *analysis.Pass, body *ast.BlockStmt, first, second ast.Stmt, if form == usePointerTo { display = usePointerTo } + message := fmt.Sprintf("%q is only used as an address by the following statement and should be inlined with %s", ident.Name, display) if len(between) > 0 { - message = fmt.Sprintf("%q is only used as an address by the statement on line %d and should be inlined with %s", - ident.Name, pass.Fset.Position(second.Pos()).Line, display) + message = fmt.Sprintf("%q is only used as an address by the statement on line %d and should be inlined with %s", ident.Name, pass.Fset.Position(second.Pos()).Line, display) } + // `v := *p` then `&v` could inline to the copy-preserving pointer.To(*p)/new(*p), but the // better form is often p itself — dropping the copy changes aliasing, so fix-pointer-copy picks // the fix: none (default) leaves the choice to a person, copy preserves the copy, share @@ -267,6 +265,7 @@ func checkPair(pass *analysis.Pass, body *ast.BlockStmt, first, second ast.Stmt, } } } + pass.Report(analysis.Diagnostic{ Pos: assign.Pos(), Message: message, @@ -308,10 +307,12 @@ func suggestedFixes(pass *analysis.Pass, assign *ast.AssignStmt, addr *ast.Unary if file == nil { return nil } + pkgName, edit, ok := pointerpkg.Ref(file) if !ok { return nil } + creation = pkgName + ".To(" + string(exprSrc) + ")" importEdit = edit } diff --git a/checks/AZG/AZG004_zero_value_init_pointer_from/AZG004.go b/checks/AZG/AZG004_zero_value_init_pointer_from/AZG004.go index dc1dc66..78aec2e 100644 --- a/checks/AZG/AZG004_zero_value_init_pointer_from/AZG004.go +++ b/checks/AZG/AZG004_zero_value_init_pointer_from/AZG004.go @@ -240,11 +240,13 @@ func suggestedFixes(pass *analysis.Pass, initStmt ast.Stmt, ifStmt *ast.IfStmt, if !ok || init.Tok != token.DEFINE || len(init.Lhs) != 1 || len(init.Rhs) != 1 { return nil } + lhs, lhsOk := init.Lhs[0].(*ast.Ident) checked, checkedOk := checkedExpr.(*ast.Ident) if !lhsOk || !checkedOk || lhs.Name != checked.Name { return nil } + fromExpr = init.Rhs[0] } diff --git a/checks/AZG/AZG005_single_use_temporary/AZG005.go b/checks/AZG/AZG005_single_use_temporary/AZG005.go index d2be75c..7463292 100644 --- a/checks/AZG/AZG005_single_use_temporary/AZG005.go +++ b/checks/AZG/AZG005_single_use_temporary/AZG005.go @@ -45,8 +45,7 @@ var maxGap int func init() { Analyzer.Flags.Init("AZG005", flag.ContinueOnError) - Analyzer.Flags.IntVar(&maxGap, "max-gap", 100, - "maximum number of source lines between the temporary's declaration and its consumer") + Analyzer.Flags.IntVar(&maxGap, "max-gap", 100, "maximum number of source lines between the temporary's declaration and its consumer") } func run(pass *analysis.Pass) (any, error) { @@ -142,9 +141,9 @@ func checkPair(pass *analysis.Pass, body *ast.BlockStmt, first, second ast.Stmt, message := fmt.Sprintf("%q is only used by the following statement and should be inlined", ident.Name) if !adjacent { - message = fmt.Sprintf("%q is only used by the statement on line %d and should be inlined", - ident.Name, pass.Fset.Position(second.Pos()).Line) + message = fmt.Sprintf("%q is only used by the statement on line %d and should be inlined", ident.Name, pass.Fset.Position(second.Pos()).Line) } + pass.Report(analysis.Diagnostic{ Pos: assign.Pos(), Message: message, diff --git a/checks/AZG/AZG006_single_use_call_argument/AZG006.go b/checks/AZG/AZG006_single_use_call_argument/AZG006.go index 38587dc..7f37ae3 100644 --- a/checks/AZG/AZG006_single_use_call_argument/AZG006.go +++ b/checks/AZG/AZG006_single_use_call_argument/AZG006.go @@ -46,12 +46,9 @@ var maxGap int func init() { Analyzer.Flags.Init("AZG006", flag.ContinueOnError) - Analyzer.Flags.IntVar(&maxGap, "max-gap", 100, - "maximum number of source lines between the variable's declaration and the consuming call") - Analyzer.Flags.BoolVar(&onlyWhenLiterals, "only-when-literals", false, - "only inline when every sibling argument is a basic literal (plain identifiers are otherwise also accepted)") - Analyzer.Flags.IntVar(&maximumArguments, "maximum-arguments", 0, - "skip calls with more than this many arguments (0 = unlimited)") + Analyzer.Flags.IntVar(&maxGap, "max-gap", 100, "maximum number of source lines between the variable's declaration and the consuming call") + Analyzer.Flags.BoolVar(&onlyWhenLiterals, "only-when-literals", false, "only inline when every sibling argument is a basic literal (plain identifiers are otherwise also accepted)") + Analyzer.Flags.IntVar(&maximumArguments, "maximum-arguments", 0, "skip calls with more than this many arguments (0 = unlimited)") } // onlyWhenLiterals restricts sibling arguments to basic literals; by default plain @@ -171,9 +168,9 @@ func checkPair(pass *analysis.Pass, body *ast.BlockStmt, first, second ast.Stmt, message := fmt.Sprintf("%q is only used by the following call and should be inlined", ident.Name) if !adjacent { - message = fmt.Sprintf("%q is only used by the call on line %d and should be inlined", - ident.Name, pass.Fset.Position(second.Pos()).Line) + message = fmt.Sprintf("%q is only used by the call on line %d and should be inlined", ident.Name, pass.Fset.Position(second.Pos()).Line) } + pass.Report(analysis.Diagnostic{ Pos: assign.Pos(), Message: message, diff --git a/checks/AZG/AZG007_redundant_zero_value_field/AZG007.go b/checks/AZG/AZG007_redundant_zero_value_field/AZG007.go index 27430cc..0ca0886 100644 --- a/checks/AZG/AZG007_redundant_zero_value_field/AZG007.go +++ b/checks/AZG/AZG007_redundant_zero_value_field/AZG007.go @@ -38,8 +38,7 @@ var checkTests bool func init() { Analyzer.Flags.Init("AZG007", flag.ContinueOnError) - Analyzer.Flags.BoolVar(&checkTests, "tests", false, - "also report in test files, where a zero entry in a test table is often meaningful (off by default)") + Analyzer.Flags.BoolVar(&checkTests, "tests", false, "also report in test files, where a zero entry in a test table is often meaningful (off by default)") } func run(pass *analysis.Pass) (any, error) { @@ -97,13 +96,12 @@ func run(pass *analysis.Pass) (any, error) { if i > 0 { prevEnd = compositeLit.Elts[i-1].End() } + prevLine := pass.Fset.Position(prevEnd).Line kvLine := pass.Fset.Position(kv.Pos()).Line leadComment := false for _, cg := range commentGroups { - if cg.Pos() > prevEnd && cg.End() < kv.Pos() && - pass.Fset.Position(cg.Pos()).Line > prevLine && - pass.Fset.Position(cg.End()).Line == kvLine-1 { + if cg.Pos() > prevEnd && cg.End() < kv.Pos() && pass.Fset.Position(cg.Pos()).Line > prevLine && pass.Fset.Position(cg.End()).Line == kvLine-1 { leadComment = true break } @@ -196,5 +194,6 @@ func isRedundantZero(pass *analysis.Pass, basic *types.Basic, value ast.Expr) bo case info&types.IsNumeric != 0: return constant.Sign(cv) == 0 } + return false } diff --git a/checks/AZG/AZG008_unchecked_nil_dereference/AZG008.go b/checks/AZG/AZG008_unchecked_nil_dereference/AZG008.go index 59df993..1e4e567 100644 --- a/checks/AZG/AZG008_unchecked_nil_dereference/AZG008.go +++ b/checks/AZG/AZG008_unchecked_nil_dereference/AZG008.go @@ -72,14 +72,10 @@ var fixWith string func init() { Analyzer.Flags.Init("AZG008", flag.ContinueOnError) - Analyzer.Flags.BoolVar(&includeParameters, "include-parameters", false, - "also report dereferences of bare pointer parameters (callers' nil-check contract is otherwise trusted)") - Analyzer.Flags.BoolVar(&checkTests, "tests", true, - "check _test.go files (false skips them)") - Analyzer.Flags.BoolVar(&reportRequestBody, "requestbody", true, - "report dereferences sent as a PUT/PATCH/POST body (never fixed); false skips them") - Analyzer.Flags.StringVar(&fixWith, "fix-with", fixPointerFrom, - "suggested-fix form: pointer.From or none") + Analyzer.Flags.BoolVar(&includeParameters, "include-parameters", false, "also report dereferences of bare pointer parameters (callers' nil-check contract is otherwise trusted)") + Analyzer.Flags.BoolVar(&checkTests, "tests", true, "check _test.go files (false skips them)") + Analyzer.Flags.BoolVar(&reportRequestBody, "requestbody", true, "report dereferences sent as a PUT/PATCH/POST body (never fixed); false skips them") + Analyzer.Flags.StringVar(&fixWith, "fix-with", fixPointerFrom, "suggested-fix form: pointer.From or none") } func run(pass *analysis.Pass) (any, error) { @@ -171,6 +167,7 @@ func checkDeref(pass *analysis.Pass, parents map[ast.Node]ast.Node, params map[t for parents[body] != nil { body = parents[body] } + sinks := map[types.Object]bool{} ast.Inspect(body, func(n ast.Node) bool { if call, ok := n.(*ast.CallExpr); ok { @@ -239,6 +236,7 @@ climb: case fixWith == fixPointerFrom: fixes = suggestedFixes(pass, parents, star) } + pass.Report(analysis.Diagnostic{ Pos: star.Pos(), Message: msg, @@ -263,10 +261,12 @@ func suggestedFixes(pass *analysis.Pass, parents map[ast.Node]ast.Node, star *as if !ok { return nil } + file := astx.EnclosingFile(pass, star.Pos()) if file == nil { return nil } + pkgName, importEdit, ok := pointerpkg.Ref(file) if !ok { return nil @@ -293,6 +293,7 @@ func suggestedFixes(pass *analysis.Pass, parents map[ast.Node]ast.Node, star *as if importEdit != nil { edits = append(edits, *importEdit) } + return []analysis.SuggestedFix{{ Message: "Replace the dereference with " + pkgName + ".From", TextEdits: edits, diff --git a/checks/AZG/AZG009_pointer_from_enum_conversion/AZG009.go b/checks/AZG/AZG009_pointer_from_enum_conversion/AZG009.go index 9086aa7..15fb177 100644 --- a/checks/AZG/AZG009_pointer_from_enum_conversion/AZG009.go +++ b/checks/AZG/AZG009_pointer_from_enum_conversion/AZG009.go @@ -32,26 +32,32 @@ func run(pass *analysis.Pass) (any, error) { if !ok || len(call.Args) != 1 { return } + conversion, ok := ast.Unparen(call.Fun).(*ast.Ident) if !ok || pass.TypesInfo.Uses[conversion] != types.Universe.Lookup("string") { return } + from, ok := ast.Unparen(call.Args[0]).(*ast.CallExpr) if !ok || len(from.Args) != 1 { return } + fun := ast.Unparen(from.Fun) if index, isIndex := fun.(*ast.IndexExpr); isIndex { fun = ast.Unparen(index.X) } + sel, ok := fun.(*ast.SelectorExpr) if !ok { return } + fn := astx.CalledFunc(pass, from) if fn == nil || fn.Name() != "From" || fn.Pkg() == nil || fn.Pkg().Path() != pointerpkg.PkgPath { return } + named, ok := types.Unalias(pass.TypesInfo.TypeOf(from)).(*types.Named) if !ok || !azuresdk.IsEnumType(named) { return @@ -76,8 +82,7 @@ func suggestedFixes(pass *analysis.Pass, call, from *ast.CallExpr, sel *ast.Sele } for _, group := range file.Comments { for _, comment := range group.List { - if comment.Pos() > call.Pos() && comment.End() < call.End() && - (comment.Pos() < from.Pos() || comment.End() > from.End()) { + if comment.Pos() > call.Pos() && comment.End() < call.End() && (comment.Pos() < from.Pos() || comment.End() > from.End()) { return nil } } diff --git a/checks/AZG/AZG010_unneeded_import_alias/AZG010.go b/checks/AZG/AZG010_unneeded_import_alias/AZG010.go index fd747ab..216ba54 100644 --- a/checks/AZG/AZG010_unneeded_import_alias/AZG010.go +++ b/checks/AZG/AZG010_unneeded_import_alias/AZG010.go @@ -67,12 +67,9 @@ var checkGenerated bool // split on a separator that it may itself contain func init() { Analyzer.Flags.Init("AZG010", flag.ContinueOnError) - Analyzer.Flags.BoolVar(&checkGenerated, "generated", false, - "also check generated files") - Analyzer.Flags.BoolVar(&ownPackage, "own-package", false, - "also report an alias that avoids the name of the file's own package") - Analyzer.Flags.BoolVar(&mismatched, "mismatched", true, - "report an alias that is the package's real name where the import path does not show it (false leaves it alone)") + Analyzer.Flags.BoolVar(&checkGenerated, "generated", false, "also check generated files") + Analyzer.Flags.BoolVar(&ownPackage, "own-package", false, "also report an alias that avoids the name of the file's own package") + Analyzer.Flags.BoolVar(&mismatched, "mismatched", true, "report an alias that is the package's real name where the import path does not show it (false leaves it alone)") Analyzer.Flags.Func("ignore", "skip imports whose package name matches this regular expression; repeat for several", func(s string) error { pattern, err := regexp.Compile(s) @@ -114,10 +111,12 @@ func run(pass *analysis.Pass) (any, error) { if spec.Name == nil || spec.Name.Name == "_" || spec.Name.Name == "." { continue } + pkgName, ok := pass.TypesInfo.Defs[spec.Name].(*types.PkgName) if !ok { continue } + alias := spec.Name.Name name := pkgName.Imported().Name() @@ -137,6 +136,7 @@ func run(pass *analysis.Pass) (any, error) { } writeName = base != name } + if alias == name && writeName { continue } @@ -169,6 +169,7 @@ func run(pass *analysis.Pass) (any, error) { clash = true } } + if clash { continue } @@ -196,6 +197,7 @@ func run(pass *analysis.Pass) (any, error) { return true }) } + if declared[name] { continue } @@ -233,6 +235,7 @@ func run(pass *analysis.Pass) (any, error) { } } } + if testDeclared[name] { continue } diff --git a/checks/AZG/AZG010_unneeded_import_alias/AZG010_test.go b/checks/AZG/AZG010_unneeded_import_alias/AZG010_test.go index c725d55..9b92cef 100644 --- a/checks/AZG/AZG010_unneeded_import_alias/AZG010_test.go +++ b/checks/AZG/AZG010_unneeded_import_alias/AZG010_test.go @@ -25,6 +25,7 @@ func TestAZG010(t *testing.T) { t.Fatal(err) } } + analysistest.Run(t, dir, Analyzer, "azg010ignore") ignore = nil @@ -33,6 +34,7 @@ func TestAZG010(t *testing.T) { t.Fatal(err) } } + analysistest.Run(t, dir, Analyzer, "azg010allow") allow = nil diff --git a/checks/AZP/AZP003_data_source_missing_properties/AZP003.go b/checks/AZP/AZP003_data_source_missing_properties/AZP003.go index fb1d3ea..63a13ef 100644 --- a/checks/AZP/AZP003_data_source_missing_properties/AZP003.go +++ b/checks/AZP/AZP003_data_source_missing_properties/AZP003.go @@ -47,8 +47,7 @@ var Analyzer = &analysis.Analyzer{ var ignoreSensitive bool func init() { - Analyzer.Flags.BoolVar(&ignoreSensitive, "ignore-sensitive", false, - "do not report resource properties marked Sensitive: true") + Analyzer.Flags.BoolVar(&ignoreSensitive, "ignore-sensitive", false, "do not report resource properties marked Sensitive: true") } var ( @@ -149,6 +148,7 @@ func run(pass *analysis.Pass) (any, error) { if !ok || reported[ds.name] { continue } + reported[ds.name] = true // per-property //azignore:AZP003 directives are honoured on the resource side only: @@ -167,6 +167,7 @@ func run(pass *analysis.Pass) (any, error) { if len(missing) == 0 { continue } + slices.Sort(missing) pass.Reportf(ds.pos, "data source %q is missing resource properties: %s", ds.name, strings.Join(missing, ", ")) @@ -193,15 +194,18 @@ func (c *collector) collectRegistrations(body *ast.BlockStmt) []entry { if ptr, ok := t.(*types.Pointer); ok { t = types.Unalias(ptr.Elem()) } + named, ok := t.(*types.Named) if !ok { return } + typeName := named.Obj().Name() name, ok := c.resourceTypeOf(typeName) if !ok { return } + entries = append(entries, entry{name: name, pos: elem.Pos(), typeName: typeName}) } @@ -295,14 +299,17 @@ func (t *treeNode) walkMissing(dsNames map[string]bool, prefix string, missing * if ignoreSensitive && child.sensitive { continue } + path := name if prefix != "" { path = prefix + "." + name } + if !dsNames[name] { *missing = append(*missing, strconv.Quote(path)) continue } + child.walkMissing(dsNames, path, missing) } } @@ -371,10 +378,12 @@ func (b *treeBuilder) build(n ast.Node, node *treeNode) { if !ok || i >= len(v.Rhs) { continue } + m, ok := types.Unalias(b.c.pass.TypesInfo.TypeOf(idx.X)).(*types.Map) if !ok || !isSchemaMap(m) { continue } + handled = true if name, ok := b.c.constantString(idx.Index); ok { if b.honorIgnores && b.c.ignoredAt(idx.Index.Pos()) { @@ -419,6 +428,7 @@ func (b *treeBuilder) hasBoolField(expr ast.Expr, field string) bool { return false } } + kv, ok := n.(*ast.KeyValueExpr) if !ok { return true @@ -429,6 +439,7 @@ func (b *treeBuilder) hasBoolField(expr ast.Expr, field string) bool { return false } } + return true }) return found @@ -442,14 +453,17 @@ func (b *treeBuilder) collectMap(lit *ast.CompositeLit, node *treeNode) { if !ok { continue } + name, ok := b.c.constantString(kv.Key) if !ok { b.clean = false continue } + if b.honorIgnores && b.c.ignoredAt(kv.Key.Pos()) { continue } + child := node.child(name) if b.hasBoolField(kv.Value, "WriteOnly") { child.writeOnly = true @@ -457,6 +471,7 @@ func (b *treeBuilder) collectMap(lit *ast.CompositeLit, node *treeNode) { if b.hasBoolField(kv.Value, "Sensitive") { child.sensitive = true } + b.build(kv.Value, child) } } @@ -473,14 +488,17 @@ func isSchemaMap(m *types.Map) bool { if ptr, isPtr := elem.(*types.Pointer); isPtr { elem = types.Unalias(ptr.Elem()) } + named, ok := elem.(*types.Named) if !ok { return false } + switch named.Obj().Name() { case "Schema", "Attribute", "Block": return true } + return false } @@ -517,14 +535,17 @@ func (c *collector) delegatesToRegistrationMethod(expr ast.Expr) bool { if !ok { return false } + sel, ok := call.Fun.(*ast.SelectorExpr) if !ok { return false } + name := sel.Sel.Name if !resourceMethods[name] && !dataSourceMethods[name] { return false } + _, isFunc := c.pass.TypesInfo.ObjectOf(sel.Sel).(*types.Func) return isFunc } @@ -562,6 +583,7 @@ func (c *collector) constantString(expr ast.Expr) (string, bool) { if !ok { return "", false } + v, ok := c.pass.TypesInfo.ObjectOf(id).(*types.Var) if !ok || v.Pkg() != c.pass.Pkg || v.Parent() != c.pass.Pkg.Scope() { return "", false diff --git a/checks/AZP/AZP004_registration_entries_sorted/AZP004.go b/checks/AZP/AZP004_registration_entries_sorted/AZP004.go index 6b58842..7c01c47 100644 --- a/checks/AZP/AZP004_registration_entries_sorted/AZP004.go +++ b/checks/AZP/AZP004_registration_entries_sorted/AZP004.go @@ -32,8 +32,7 @@ var checkGenerated bool func init() { Analyzer.Flags.Init("AZP004", flag.ContinueOnError) - Analyzer.Flags.BoolVar(&checkGenerated, "generated", true, - "check generated registration_gen.go files (false skips them)") + Analyzer.Flags.BoolVar(&checkGenerated, "generated", true, "check generated registration_gen.go files (false skips them)") } func run(pass *analysis.Pass) (any, error) { @@ -91,6 +90,7 @@ func analyzeRegistrationMethod(pass *analysis.Pass, file *ast.File, funcDecl *as if !ok { return } + signature, ok := fn.Type().(*types.Signature) if !ok { return @@ -110,12 +110,14 @@ func analyzeRegistrationMethod(pass *analysis.Pass, file *ast.File, funcDecl *as if literalType == nil { return true } + for result := range signature.Results().Variables() { if types.Identical(literalType, result.Type()) { validateSorting(pass, file, compositeLit) break } } + return true }) } @@ -237,8 +239,7 @@ func sortFix(pass *analysis.Pass, file *ast.File, compositeLit *ast.CompositeLit if section[0] == elts[0] && tf.Line(compositeLit.Lbrace) == tf.Line(elts[0].Pos()) { return nil } - if section[len(section)-1] == elts[len(elts)-1] && - tf.Line(compositeLit.Rbrace) == tf.Line(elts[len(elts)-1].End()) { + if section[len(section)-1] == elts[len(elts)-1] && tf.Line(compositeLit.Rbrace) == tf.Line(elts[len(elts)-1].End()) { return nil } } @@ -251,6 +252,7 @@ func sortFix(pass *analysis.Pass, file *ast.File, compositeLit *ast.CompositeLit if hasSpanningComment(file.Comments, tf, section) { return nil } + edit, ok := sortSectionEdit(tf, content, file.Comments, compositeLit, section, isMap) if !ok { return nil @@ -293,9 +295,7 @@ func attachedLeadingComment(comments []*ast.CommentGroup, tf *token.File, compos } for _, comment := range comments { - if comment.Pos() > previousEnd && comment.End() < entry.Pos() && - tf.Line(comment.Pos()) == tf.Line(previousEnd)+1 && - tf.Line(entry.Pos()) == tf.Line(comment.End())+1 { + if comment.Pos() > previousEnd && comment.End() < entry.Pos() && tf.Line(comment.Pos()) == tf.Line(previousEnd)+1 && tf.Line(entry.Pos()) == tf.Line(comment.End())+1 { return comment } } @@ -324,10 +324,12 @@ func sortSectionEdit(tf *token.File, content []byte, comments []*ast.CommentGrou if leadingComment := attachedLeadingComment(comments, tf, compositeLit, elt); leadingComment != nil { startLine = tf.Line(leadingComment.Pos()) } + endLine := tf.Line(elt.End()) if i > 0 && startLine <= prevEndLine { return analysis.TextEdit{}, false // entries share a line; not safely reorderable } + prevEndLine = endLine lineStart := tf.LineStart(startLine) @@ -335,6 +337,7 @@ func sortSectionEdit(tf *token.File, content []byte, comments []*ast.CommentGrou if i == 0 { editStart = lineStart } + editEnd = lineEnd blocks[i] = block{key: key, text: content[tf.Offset(lineStart):tf.Offset(lineEnd)]} } diff --git a/checks/AZP/AZP005_case_insensitive_segments_feature_flag/AZP005.go b/checks/AZP/AZP005_case_insensitive_segments_feature_flag/AZP005.go index 4029140..18c2211 100644 --- a/checks/AZP/AZP005_case_insensitive_segments_feature_flag/AZP005.go +++ b/checks/AZP/AZP005_case_insensitive_segments_feature_flag/AZP005.go @@ -43,8 +43,7 @@ func run(pass *analysis.Pass) (any, error) { continue } - pass.Reportf(assign.Pos(), - "TreatUserSpecifiedSegmentsAsCaseInsensitive must not be set, the case-aware comparisons feature is not ready for use") + pass.Reportf(assign.Pos(), "TreatUserSpecifiedSegmentsAsCaseInsensitive must not be set, the case-aware comparisons feature is not ready for use") } }) diff --git a/checks/AZR/AZR001_set_id_dereferenced_pointer/AZR001.go b/checks/AZR/AZR001_set_id_dereferenced_pointer/AZR001.go index ff8315d..5cb3424 100644 --- a/checks/AZR/AZR001_set_id_dereferenced_pointer/AZR001.go +++ b/checks/AZR/AZR001_set_id_dereferenced_pointer/AZR001.go @@ -47,8 +47,7 @@ func run(pass *analysis.Pass) (any, error) { return } - pass.Reportf(call.Pos(), - "SetId should not be passed a dereferenced pointer, use a generated Resource ID Formatter/Parser and id.ID()") + pass.Reportf(call.Pos(), "SetId should not be passed a dereferenced pointer, use a generated Resource ID Formatter/Parser and id.ID()") }) return nil, nil diff --git a/checks/AZR/AZR002_combined_create_update_method/AZR002.go b/checks/AZR/AZR002_combined_create_update_method/AZR002.go index 848a3e7..5cce00d 100644 --- a/checks/AZR/AZR002_combined_create_update_method/AZR002.go +++ b/checks/AZR/AZR002_combined_create_update_method/AZR002.go @@ -47,8 +47,7 @@ func run(pass *analysis.Pass) (any, error) { return } - pass.Reportf(kv.Pos(), - "new resources should use separate Create and Update methods instead of a combined CreateUpdate method") + pass.Reportf(kv.Pos(), "new resources should use separate Create and Update methods instead of a combined CreateUpdate method") }) return nil, nil diff --git a/checks/AZR/AZR003_resource_data_get_in_delete/AZR003.go b/checks/AZR/AZR003_resource_data_get_in_delete/AZR003.go index 72a5434..e9b9fac 100644 --- a/checks/AZR/AZR003_resource_data_get_in_delete/AZR003.go +++ b/checks/AZR/AZR003_resource_data_get_in_delete/AZR003.go @@ -69,8 +69,7 @@ func checkUntypedDelete(pass *analysis.Pass, fn *ast.FuncDecl) { return true } - pass.Reportf(call.Pos(), - "%s.Get should not be used within a Delete function as it does not work as expected during deletion", dataParam) + pass.Reportf(call.Pos(), "%s.Get should not be used within a Delete function as it does not work as expected during deletion", dataParam) return true }) } @@ -93,9 +92,11 @@ func checkTypedDelete(pass *analysis.Pass, fn *ast.FuncDecl) { default: return true } + if len(names) != len(values) { return true } + for i, v := range values { sel, ok := ast.Unparen(v).(*ast.SelectorExpr) if !ok || sel.Sel.Name != "ResourceData" { @@ -107,6 +108,7 @@ func checkTypedDelete(pass *analysis.Pass, fn *ast.FuncDecl) { } } } + return true }) @@ -126,15 +128,14 @@ func checkTypedDelete(pass *analysis.Pass, fn *ast.FuncDecl) { if x.Sel.Name != "ResourceData" { return true } - pass.Reportf(call.Pos(), - "ResourceData.Get should not be used within a Delete function as it does not work as expected during deletion") + pass.Reportf(call.Pos(), "ResourceData.Get should not be used within a Delete function as it does not work as expected during deletion") case *ast.Ident: if !aliases[pass.TypesInfo.ObjectOf(x)] { return true } - pass.Reportf(call.Pos(), - "%s.Get should not be used within a Delete function as it does not work as expected during deletion", x.Name) + pass.Reportf(call.Pos(), "%s.Get should not be used within a Delete function as it does not work as expected during deletion", x.Name) } + return true }) } diff --git a/checks/AZR/AZR004_resource_id_equality_comparison/AZR004.go b/checks/AZR/AZR004_resource_id_equality_comparison/AZR004.go index a7483be..7978f72 100644 --- a/checks/AZR/AZR004_resource_id_equality_comparison/AZR004.go +++ b/checks/AZR/AZR004_resource_id_equality_comparison/AZR004.go @@ -42,8 +42,7 @@ func run(pass *analysis.Pass) (any, error) { return } - pass.Reportf(cmp.Pos(), - "Resource IDs should not be compared with == or !=, use resourceids.Match instead") + pass.Reportf(cmp.Pos(), "Resource IDs should not be compared with == or !=, use resourceids.Match instead") }) return nil, nil diff --git a/checks/AZR/AZR006_stop_context_without_timeouts/AZR006.go b/checks/AZR/AZR006_stop_context_without_timeouts/AZR006.go index 88c5eeb..f480f2a 100644 --- a/checks/AZR/AZR006_stop_context_without_timeouts/AZR006.go +++ b/checks/AZR/AZR006_stop_context_without_timeouts/AZR006.go @@ -53,8 +53,7 @@ func run(pass *analysis.Pass) (any, error) { return } - pass.Reportf(assign.Pos(), - "use a timeouts-wrapped StopContext (timeouts.ForCreate/ForCreateUpdate/ForRead/ForUpdate/ForDelete) so Custom Timeouts are supported, instead of assigning ctx from meta directly") + pass.Reportf(assign.Pos(), "use a timeouts-wrapped StopContext (timeouts.ForCreate/ForCreateUpdate/ForRead/ForUpdate/ForDelete) so Custom Timeouts are supported, instead of assigning ctx from meta directly") }) return nil, nil diff --git a/checks/AZR/AZR007_state_change_conf_custom_poller/AZR007.go b/checks/AZR/AZR007_state_change_conf_custom_poller/AZR007.go index 76bf5e5..1340816 100644 --- a/checks/AZR/AZR007_state_change_conf_custom_poller/AZR007.go +++ b/checks/AZR/AZR007_state_change_conf_custom_poller/AZR007.go @@ -60,8 +60,7 @@ func run(pass *analysis.Pass) (any, error) { return } - pass.Reportf(lit.Pos(), - "prefer a Custom Poller over StateChangeConf") + pass.Reportf(lit.Pos(), "prefer a Custom Poller over StateChangeConf") }) return nil, nil diff --git a/checks/AZR/AZR008_flatten_returns_nil_slice/AZR008.go b/checks/AZR/AZR008_flatten_returns_nil_slice/AZR008.go index b8d6da2..da68efc 100644 --- a/checks/AZR/AZR008_flatten_returns_nil_slice/AZR008.go +++ b/checks/AZR/AZR008_flatten_returns_nil_slice/AZR008.go @@ -91,6 +91,7 @@ func run(pass *analysis.Pass) (any, error) { results = append(results, withName) } } + if !haveContainer { return } @@ -144,6 +145,7 @@ func checkExplicitReturn(pass *analysis.Pass, body *ast.BlockStmt, retStmt *ast. if !isProvablyNil(pass, body, res, retStmt.Pos(), namedResults) { continue } + kinds = append(kinds, results[i].kind) edit := analysis.TextEdit{ Pos: res.Pos(), @@ -197,6 +199,7 @@ func checkNakedReturn(pass *analysis.Pass, body *ast.BlockStmt, retStmt *ast.Ret } values = append(values, res.name.Name()) } + if len(kinds) == 0 { return } @@ -240,10 +243,12 @@ func isProvablyNil(pass *analysis.Pass, body *ast.BlockStmt, expr ast.Expr, retP if astx.IsNilValue(pass, expr) { return true } + ident, ok := ast.Unparen(expr).(*ast.Ident) if !ok { return false } + obj := pass.TypesInfo.Uses[ident] if obj == nil { return false @@ -251,6 +256,7 @@ func isProvablyNil(pass *analysis.Pass, body *ast.BlockStmt, expr ast.Expr, retP if !namedResults[obj] && !isZeroValueDeclared(pass, body, obj) { return false } + return isUnassignedBefore(pass, body, obj, retPos) } @@ -321,20 +327,24 @@ func declarationLineEdit(pass *analysis.Pass, body *ast.BlockStmt, obj types.Obj if !ok { return true } + gen, ok := decl.Decl.(*ast.GenDecl) if !ok || len(gen.Specs) != 1 { return true } + spec, ok := gen.Specs[0].(*ast.ValueSpec) if !ok || len(spec.Names) != 1 || pass.TypesInfo.Defs[spec.Names[0]] != obj { return true } + found = true tf := pass.Fset.File(decl.Pos()) delEnd := decl.End() if endLine := tf.Line(decl.End()); endLine+1 <= tf.LineCount() { delEnd = tf.LineStart(endLine + 1) } + edit = &analysis.TextEdit{Pos: tf.LineStart(tf.Line(decl.Pos())), End: delEnd} return false }) diff --git a/checks/AZR/AZR009_lifecycle_logging/AZR009.go b/checks/AZR/AZR009_lifecycle_logging/AZR009.go index 8cc704c..5fcdf8e 100644 --- a/checks/AZR/AZR009_lifecycle_logging/AZR009.go +++ b/checks/AZR/AZR009_lifecycle_logging/AZR009.go @@ -63,22 +63,27 @@ func run(pass *analysis.Pass) (any, error) { if !ok { return true } + call, ok := stmt.X.(*ast.CallExpr) if !ok || !isLogCall(pass, call) || len(call.Args) == 0 { return true } + lit, ok := call.Args[0].(*ast.BasicLit) if !ok || lit.Kind != token.STRING { return true } + msg, err := strconv.Unquote(lit.Value) if err != nil { return true } + msg = levelPrefix.ReplaceAllString(strings.TrimSpace(msg), "") if narration.MatchString(msg) && !keep.MatchString(msg) { byFile[file] = append(byFile[file], finding{stmt: stmt, call: call}) } + return true }) } @@ -115,6 +120,7 @@ func run(pass *analysis.Pass) (any, error) { }) } } + return nil, nil } diff --git a/checks/AZR/AZR010_flatten_handles_nil_input/AZR010.go b/checks/AZR/AZR010_flatten_handles_nil_input/AZR010.go index e765c1a..cf8d022 100644 --- a/checks/AZR/AZR010_flatten_handles_nil_input/AZR010.go +++ b/checks/AZR/AZR010_flatten_handles_nil_input/AZR010.go @@ -168,10 +168,12 @@ func run(pass *analysis.Pass) (any, error) { if !ok { return true } + fn := astx.CalledFunc(pass, call) if fn == nil || !isFlatten(fn.Name()) { return true } + for i, arg := range call.Args { t := pass.TypesInfo.TypeOf(arg) if t == nil { @@ -180,14 +182,17 @@ func run(pass *analysis.Pass) (any, error) { if _, isPtr := t.Underlying().(*types.Pointer); !isPtr { continue } + key, ok := nilguard.PathKey(pass, arg) if !ok { continue } + ifs, keys := guardingIf(pass, parents, call, key) if ifs == nil || otherUse(pass, parents, ifs, keys) { continue } + src := types.ExprString(ast.Unparen(arg)) msg := "`" + fn.Name() + "(" + src + ")` is called under a nil check on `" + src + "` - handle nil inside the flatten function instead" var fixes []analysis.SuggestedFix @@ -195,8 +200,10 @@ func run(pass *analysis.Pass) (any, error) { msg = "`" + fn.Name() + "` already handles a nil `" + src + "` - drop the nil check" fixes = dropCheck(pass, ifs) } + pass.Report(analysis.Diagnostic{Pos: call.Pos(), Message: msg, SuggestedFixes: fixes}) } + return true }) }) @@ -229,8 +236,7 @@ func guardingIf(pass *analysis.Pass, parents map[ast.Node]ast.Node, call ast.Nod keys = append(keys, lk) } } - if (ifs.Body == child && nilCompare(pass, ifs.Cond, keys, token.NEQ, token.LAND)) || - (ifs.Else == child && nilCompare(pass, ifs.Cond, keys, token.EQL, token.LOR)) { + if (ifs.Body == child && nilCompare(pass, ifs.Cond, keys, token.NEQ, token.LAND)) || (ifs.Else == child && nilCompare(pass, ifs.Cond, keys, token.EQL, token.LOR)) { return ifs, keys } } @@ -250,12 +256,14 @@ func nilCompare(pass *analysis.Pass, cond ast.Expr, keys []string, cmp, join tok if c.Op != cmp { return false } + other := c.X if astx.IsNilValue(pass, c.X) { other = c.Y } else if !astx.IsNilValue(pass, c.Y) { return false } + k, ok := nilguard.PathKey(pass, other) return ok && slices.Contains(keys, k) } @@ -274,6 +282,7 @@ func otherUse(pass *analysis.Pass, parents map[ast.Node]ast.Node, ifs *ast.IfStm if !ok || found { return !found } + k, ok := nilguard.PathKey(pass, e) if !ok { return true @@ -290,6 +299,7 @@ func otherUse(pass *analysis.Pass, parents map[ast.Node]ast.Node, ifs *ast.IfStm } } } + found = true return false }) @@ -307,16 +317,19 @@ func dropCheck(pass *analysis.Pass, ifs *ast.IfStmt) []analysis.SuggestedFix { if !ok || cmp.Op != token.NEQ || ifs.Init != nil || ifs.Else != nil || len(ifs.Body.List) != 1 { return nil } + stmt := ifs.Body.List[0] src, ok := astx.SourceText(pass, ifs) if !ok { return nil } + rbrace := int(ifs.Body.Rbrace - ifs.Pos()) lineStart := strings.LastIndex(string(src[:rbrace]), "\n") if lineStart < 0 { return nil // single-line if: nothing to unwrap cleanly } + edits := []analysis.TextEdit{ {Pos: ifs.Pos(), End: stmt.Pos()}, {Pos: ifs.Pos() + token.Pos(lineStart), End: ifs.Body.Rbrace + 1}, @@ -332,5 +345,6 @@ func dropCheck(pass *analysis.Pass, ifs *ast.IfStmt) []analysis.SuggestedFix { } off += 2 + next } + return []analysis.SuggestedFix{{Message: "Drop the nil check", TextEdits: edits}} } diff --git a/checks/AZS/AZS001_typed_sdk_model_64bit_types/AZS001.go b/checks/AZS/AZS001_typed_sdk_model_64bit_types/AZS001.go index da7c4d9..cb1d042 100644 --- a/checks/AZS/AZS001_typed_sdk_model_64bit_types/AZS001.go +++ b/checks/AZS/AZS001_typed_sdk_model_64bit_types/AZS001.go @@ -76,8 +76,7 @@ func checkModelStruct(pass *analysis.Pass, modelName string, structType *ast.Str } for _, name := range fieldNames(field) { - pass.Reportf(field.Pos(), - "property %s in model %s should be type %s, got %s", name, modelName, want, got) + pass.Reportf(field.Pos(), "property %s in model %s should be type %s, got %s", name, modelName, want, got) } } } diff --git a/checks/AZS/AZS002_schema_default_type_mismatch/AZS002.go b/checks/AZS/AZS002_schema_default_type_mismatch/AZS002.go index 49f1ae5..e21d8d3 100644 --- a/checks/AZS/AZS002_schema_default_type_mismatch/AZS002.go +++ b/checks/AZS/AZS002_schema_default_type_mismatch/AZS002.go @@ -81,6 +81,7 @@ func run(pass *analysis.Pass) (any, error) { defaultExpr = kv.Value } } + if typeExpr == nil || defaultExpr == nil { return } @@ -92,6 +93,7 @@ func run(pass *analysis.Pass) (any, error) { case *ast.Ident: schemaType = v.Name } + compatible, ok := compatibleKinds[schemaType] if !ok { return @@ -102,14 +104,13 @@ func run(pass *analysis.Pass) (any, error) { if value == nil { return } + kindName, ok := kindNames[value.Kind()] if !ok || slices.Contains(compatible, value.Kind()) { return } - pass.Reportf(defaultExpr.Pos(), - "schema Default value type %s does not match the declared Type %s", - kindName, schemaType) + pass.Reportf(defaultExpr.Pos(), "schema Default value type %s does not match the declared Type %s", kindName, schemaType) }) return nil, nil diff --git a/checks/AZS/AZS003_schema_allows_empty_block/AZS003.go b/checks/AZS/AZS003_schema_allows_empty_block/AZS003.go index d92e986..321efd6 100644 --- a/checks/AZS/AZS003_schema_allows_empty_block/AZS003.go +++ b/checks/AZS/AZS003_schema_allows_empty_block/AZS003.go @@ -53,6 +53,7 @@ func run(pass *analysis.Pass) (any, error) { case *ast.Ident: schemaType = v.Name } + if schemaType != "TypeList" { return } @@ -66,10 +67,12 @@ func run(pass *analysis.Pass) (any, error) { if !ok || ref.Op != token.AND { return } + resource, ok := ref.X.(*ast.CompositeLit) if !ok || !tf.IsSchemaHelperType(pass, resource, "Resource") { return } + properties, ok := compositeLitFields(resource)["Schema"].(*ast.CompositeLit) if !ok || len(properties.Elts) == 0 { return @@ -80,25 +83,25 @@ func run(pass *analysis.Pass) (any, error) { if !ok { return } + value := kv.Value if addr, isAddr := value.(*ast.UnaryExpr); isAddr && addr.Op == token.AND { value = addr.X } + // a property defined elsewhere (variable/function) cannot be inspected - stay quiet property, ok := value.(*ast.CompositeLit) if !ok { return } + propertyFields := compositeLitFields(property) - if astx.IsTrueConstant(pass, propertyFields["Required"]) || - isSet(propertyFields["Default"]) || isSet(propertyFields["DefaultFunc"]) || - isSet(propertyFields["AtLeastOneOf"]) || isSet(propertyFields["ExactlyOneOf"]) { + if astx.IsTrueConstant(pass, propertyFields["Required"]) || isSet(propertyFields["Default"]) || isSet(propertyFields["DefaultFunc"]) || isSet(propertyFields["AtLeastOneOf"]) || isSet(propertyFields["ExactlyOneOf"]) { return } } - pass.Reportf(cl.Pos(), - "schema allows an empty block as every property is optional with no default - add AtLeastOneOf/ExactlyOneOf constraints, a Required property, or a Default so empty blocks cannot crash expand functions or cause spurious diffs") + pass.Reportf(cl.Pos(), "schema allows an empty block as every property is optional with no default - add AtLeastOneOf/ExactlyOneOf constraints, a Required property, or a Default so empty blocks cannot crash expand functions or cause spurious diffs") }) return nil, nil diff --git a/checks/AZS/AZS004_enum_validation_possible_values/AZS004.go b/checks/AZS/AZS004_enum_validation_possible_values/AZS004.go index 0021449..3b8ec6d 100644 --- a/checks/AZS/AZS004_enum_validation_possible_values/AZS004.go +++ b/checks/AZS/AZS004_enum_validation_possible_values/AZS004.go @@ -44,10 +44,8 @@ var ( ) func init() { - Analyzer.Flags.BoolVar(&allowMissingValues, "allow-missing-values", false, - "do not report in-place validation arrays that are missing enum values (deliberate subsets)") - Analyzer.Flags.BoolVar(&allowExtraValues, "allow-extra-values", false, - "do not report in-place validation arrays containing values that are not part of the enum (deliberate supersets)") + Analyzer.Flags.BoolVar(&allowMissingValues, "allow-missing-values", false, "do not report in-place validation arrays that are missing enum values (deliberate subsets)") + Analyzer.Flags.BoolVar(&allowExtraValues, "allow-extra-values", false, "do not report in-place validation arrays containing values that are not part of the enum (deliberate supersets)") } func run(pass *analysis.Pass) (any, error) { @@ -65,6 +63,7 @@ func run(pass *analysis.Pass) (any, error) { if !ok { return } + validationPkg := stringInSliceValidationPkg(pass, call) if validationPkg == nil { return @@ -115,6 +114,7 @@ func run(pass *analysis.Pass) (any, error) { for _, v := range values { isValue[v.value] = true } + var extra []string for _, v := range coveredOrder { if !isValue[v] { @@ -131,16 +131,14 @@ func run(pass *analysis.Pass) (any, error) { // internal/tf/validation), advise calling that directly; otherwise bridge with // go-azure-helpers' generic enum-slice conversion. if hasStringInEnumSlice(validationPkg) { - helperExpr = fmt.Sprintf("%s.StringInEnumSlice(%s, %s)", - validationPkg.Name(), helperExpr, types.ExprString(call.Args[1])) + helperExpr = fmt.Sprintf("%s.StringInEnumSlice(%s, %s)", validationPkg.Name(), helperExpr, types.ExprString(call.Args[1])) } else { helperExpr = fmt.Sprintf("pointer.FromEnumSlice(pointer.To(%s))", helperExpr) } } + if len(missing) == 0 && len(extra) == 0 { - pass.Reportf(lit.Pos(), - "enum validation for %s.%s lists every value manually; use %s so new values are picked up automatically", - pkgName, enum.Obj().Name(), helperExpr) + pass.Reportf(lit.Pos(), "enum validation for %s.%s lists every value manually; use %s so new values are picked up automatically", pkgName, enum.Obj().Name(), helperExpr) return } @@ -160,9 +158,8 @@ func run(pass *analysis.Pass) (any, error) { // a plain swap to the helper would drop the extras, so the advice must keep them advice += ", appending any deliberate extras" } - pass.Reportf(lit.Pos(), - "enum validation for %s.%s %s; %s", - pkgName, enum.Obj().Name(), strings.Join(clauses, " and "), advice) + + pass.Reportf(lit.Pos(), "enum validation for %s.%s %s; %s", pkgName, enum.Obj().Name(), strings.Join(clauses, " and "), advice) }) return nil, nil @@ -275,6 +272,7 @@ func appendEnumConstTypes(pass *analysis.Pass, enums []*types.Named, elem ast.Ex return true } } + enums = append(enums, named) return true }) @@ -307,14 +305,17 @@ func enumValues(named *types.Named) (helper string, typed bool, values []enumVal if !ok { continue } + sig, ok := fn.Type().(*types.Signature) if !ok || sig.Params().Len() != 0 || sig.Results().Len() != 1 { continue } + slice, ok := sig.Results().At(0).Type().(*types.Slice) if !ok { continue } + switch { case types.Identical(slice.Elem(), types.Typ[types.String]): typed = false @@ -323,9 +324,11 @@ func enumValues(named *types.Named) (helper string, typed bool, values []enumVal default: continue } + helper = candidate break } + if helper == "" { return "", false, nil } diff --git a/checks/AZS/AZS007_optional_computed_missing_comment/AZS007.go b/checks/AZS/AZS007_optional_computed_missing_comment/AZS007.go index de9732a..cd80103 100644 --- a/checks/AZS/AZS007_optional_computed_missing_comment/AZS007.go +++ b/checks/AZS/AZS007_optional_computed_missing_comment/AZS007.go @@ -32,8 +32,7 @@ var excludePackages string func init() { Analyzer.Flags.Init("AZS007", flag.ContinueOnError) - Analyzer.Flags.StringVar(&excludePackages, "exclude-packages", "", - "comma-separated list of package names to skip") + Analyzer.Flags.StringVar(&excludePackages, "exclude-packages", "", "comma-separated list of package names to skip") } const failureMessage = "schema field has both Optional and Computed but is missing a '// Note: O+C because ...' comment between the two fields" diff --git a/checks/AZS/AZS009_computed_only_field_input_attributes/AZS009.go b/checks/AZS/AZS009_computed_only_field_input_attributes/AZS009.go index b173945..bb26594 100644 --- a/checks/AZS/AZS009_computed_only_field_input_attributes/AZS009.go +++ b/checks/AZS/AZS009_computed_only_field_input_attributes/AZS009.go @@ -65,10 +65,12 @@ func run(pass *analysis.Pass) (any, error) { if !push { return false } + cl, ok := n.(*ast.CompositeLit) if !ok || visited[cl] || !tf.IsSchemaHelperType(pass, cl, "Schema") { return true } + kvs := keyValues(cl) switch { case astx.IsTrueConstant(pass, value(kvs, "Computed")) && @@ -78,6 +80,7 @@ func run(pass *analysis.Pass) (any, error) { case inAttributesMethod(pass, stack): check(pass, cl, visited, inAttributes, true) } + return true }) @@ -123,6 +126,7 @@ func check(pass *analysis.Pass, cl *ast.CompositeLit, visited map[*ast.Composite SuggestedFixes: []analysis.SuggestedFix{{Message: "Remove " + name, TextEdits: []analysis.TextEdit{astx.DeleteLine(pass, kv)}}}, }) } + if kv := kvs["ConfigMode"]; kv != nil && isConfigModeBlock(pass, kv.Value) { pass.Report(analysis.Diagnostic{ Pos: kv.Key.Pos(), @@ -135,6 +139,7 @@ func check(pass *analysis.Pass, cl *ast.CompositeLit, visited map[*ast.Composite if !ok { return } + switch { case tf.IsSchemaHelperType(pass, elem, "Schema"): check(pass, elem, visited, onElem, false) @@ -179,14 +184,17 @@ func inAttributesMethod(pass *analysis.Pass, stack []ast.Node) bool { if fn.Name.Name != "Attributes" || fn.Recv == nil || fn.Type.Results == nil || len(fn.Type.Results.List) != 1 { return false } + m, ok := types.Unalias(pass.TypesInfo.TypeOf(fn.Type.Results.List[0].Type)).(*types.Map) if !ok { return false } + ptr, ok := types.Unalias(m.Elem()).(*types.Pointer) if !ok { return false } + named, ok := types.Unalias(ptr.Elem()).(*types.Named) return ok && named.Obj().Name() == "Schema" && named.Obj().Pkg() != nil && named.Obj().Pkg().Name() == "schema" } @@ -248,10 +256,12 @@ func isConfigModeBlock(pass *analysis.Pass, e ast.Expr) bool { if tv.Value == nil || tv.Value.Kind() != constant.Int { return false } + named, ok := types.Unalias(tv.Type).(*types.Named) if !ok || named.Obj().Name() != "SchemaConfigMode" { return false } + v, _ := constant.Int64Val(tv.Value) return v == 2 // SchemaConfigModeBlock } diff --git a/checks/AZT/AZT001_acceptance_test_external_package/AZT001.go b/checks/AZT/AZT001_acceptance_test_external_package/AZT001.go index 3ebaa47..961bb03 100644 --- a/checks/AZT/AZT001_acceptance_test_external_package/AZT001.go +++ b/checks/AZT/AZT001_acceptance_test_external_package/AZT001.go @@ -52,12 +52,12 @@ func run(pass *analysis.Pass) (any, error) { break } } + if !hasAccFunc { continue } - pass.Reportf(file.Name.Pos(), - "acceptance test files must use a _test package to prevent circular dependencies") + pass.Reportf(file.Name.Pos(), "acceptance test files must use a _test package to prevent circular dependencies") } return nil, nil diff --git a/checks/AZT/AZT002_credentials_from_environment/AZT002.go b/checks/AZT/AZT002_credentials_from_environment/AZT002.go index 57a00f5..5f2fc4d 100644 --- a/checks/AZT/AZT002_credentials_from_environment/AZT002.go +++ b/checks/AZT/AZT002_credentials_from_environment/AZT002.go @@ -62,8 +62,7 @@ func run(pass *analysis.Pass) (any, error) { return true } - pass.Reportf(call.Pos(), - "tests should not obtain credentials via os.Getenv(%q), create an azurerm_user_assigned_identity with minimal permissions as part of the test configuration instead", name) + pass.Reportf(call.Pos(), "tests should not obtain credentials via os.Getenv(%q), create an azurerm_user_assigned_identity with minimal permissions as part of the test configuration instead", name) return true }) } diff --git a/checks/AZV/AZV001_error_should_describe_expected_format/AZV001.go b/checks/AZV/AZV001_error_should_describe_expected_format/AZV001.go index b37d04c..759ba4e 100644 --- a/checks/AZV/AZV001_error_should_describe_expected_format/AZV001.go +++ b/checks/AZV/AZV001_error_should_describe_expected_format/AZV001.go @@ -46,8 +46,7 @@ func run(pass *analysis.Pass) (any, error) { } if strings.Contains(value, "invalid format of ") { - pass.Reportf(lit.Pos(), - "unclear error message: describe the expected format instead of 'invalid format of'") + pass.Reportf(lit.Pos(), "unclear error message: describe the expected format instead of 'invalid format of'") } }) diff --git a/checks/azignore/azignore.go b/checks/azignore/azignore.go index 411813f..ee73124 100644 --- a/checks/azignore/azignore.go +++ b/checks/azignore/azignore.go @@ -33,6 +33,7 @@ func ParseDirective(text string) (rules []string, reason string, ok bool) { if !strings.HasPrefix(body, prefix) { return nil, "", false } + body = strings.TrimSpace(strings.TrimPrefix(body, prefix)) list := ruleList.FindString(body) @@ -90,6 +91,7 @@ func Lines(pass *analysis.Pass, name string) map[string]map[int]bool { if ignored[pos.Filename] == nil { ignored[pos.Filename] = map[int]bool{} } + ignored[pos.Filename][pos.Line] = true ignored[pos.Filename][pos.Line+1] = true } diff --git a/checks/azignore/azignore_test.go b/checks/azignore/azignore_test.go index 50f5175..189a2ce 100644 --- a/checks/azignore/azignore_test.go +++ b/checks/azignore/azignore_test.go @@ -55,8 +55,7 @@ func TestParseDirective(t *testing.T) { rules, reason, ok := ParseDirective(tc.text) if ok != tc.ok || reason != tc.reason || !slices.Equal(rules, tc.rules) { - t.Fatalf("ParseDirective(%q) = %v, %q, %v; want %v, %q, %v", - tc.text, rules, reason, ok, tc.rules, tc.reason, tc.ok) + t.Fatalf("ParseDirective(%q) = %v, %q, %v; want %v, %q, %v", tc.text, rules, reason, ok, tc.rules, tc.reason, tc.ok) } }) } diff --git a/checks/checks.go b/checks/checks.go index c52a78c..cb716ce 100644 --- a/checks/checks.go +++ b/checks/checks.go @@ -20,13 +20,4 @@ import ( // '//azignore: - ' comment directives — except AZG000, which polices the // directives themselves and is deliberately unwrapped (and kept out of AZG.Checks) so a // bare directive cannot suppress the report about itself by listing AZG000. -var All = append(azignore.Wrap(slices.Concat( - AZC.Checks, - AZD.Checks, - AZG.Checks, - AZP.Checks, - AZR.Checks, - AZS.Checks, - AZT.Checks, - AZV.Checks, -)), AZG000.Analyzer) +var All = append(azignore.Wrap(slices.Concat(AZC.Checks, AZD.Checks, AZG.Checks, AZP.Checks, AZR.Checks, AZS.Checks, AZT.Checks, AZV.Checks)), AZG000.Analyzer) diff --git a/lib/astx/ast.go b/lib/astx/ast.go index ae4f57f..8ccc72d 100644 --- a/lib/astx/ast.go +++ b/lib/astx/ast.go @@ -139,6 +139,7 @@ func UnsafeToMovePast(pass *analysis.Pass, expr ast.Expr, stmts []ast.Stmt) bool return true } } + return false } @@ -170,16 +171,18 @@ func callMutables(pass *analysis.Pass, n ast.Node) map[*types.Var]bool { if tv, ok := pass.TypesInfo.Types[call.Fun]; ok && tv.IsType() { return true // a conversion copies its operand } + if sel, ok := ast.Unparen(call.Fun).(*ast.SelectorExpr); ok { - if fn, ok := pass.TypesInfo.Uses[sel.Sel].(*types.Func); ok && fn.Pkg() != nil && - fn.Pkg().Path() == pointerpkg.PkgPath { + if fn, ok := pass.TypesInfo.Uses[sel.Sel].(*types.Func); ok && fn.Pkg() != nil && fn.Pkg().Path() == pointerpkg.PkgPath { return true // pure: reads through its arguments, never writes } mark(sel.X) // method receiver } + for _, arg := range call.Args { mark(arg) } + return true }) return mutables @@ -264,14 +267,17 @@ func SourceText(pass *analysis.Pass, node ast.Node) ([]byte, bool) { if tf == nil { return nil, false } + content, err := pass.ReadFile(tf.Name()) if err != nil { return nil, false } + start, end := tf.Offset(node.Pos()), tf.Offset(node.End()) if start < 0 || end > len(content) || start > end { return nil, false } + return content[start:end], true } @@ -285,6 +291,7 @@ func CalledFunc(pass *analysis.Pass, call *ast.CallExpr) *types.Func { case *ast.IndexListExpr: fun = ix.X } + var id *ast.Ident switch f := fun.(type) { case *ast.Ident: @@ -294,6 +301,7 @@ func CalledFunc(pass *analysis.Pass, call *ast.CallExpr) *types.Func { default: return nil } + fn, _ := pass.TypesInfo.Uses[id].(*types.Func) return fn } diff --git a/lib/facts/facts.go b/lib/facts/facts.go index bbb03b4..6afd895 100644 --- a/lib/facts/facts.go +++ b/lib/facts/facts.go @@ -34,10 +34,12 @@ func (s *Set[T, PT]) Get(fn *types.Func) T { if s == nil || fn == nil { return v } + fn = fn.Origin() // instantiated generics resolve to their declaration if fn.Pkg() == s.pass.Pkg { return s.local[fn] } + s.pass.ImportObjectFact(fn, PT(&v)) return v } @@ -69,6 +71,7 @@ func Run[T comparable, PT interface { if !ok { return s, nil } + type candidate struct { fn *types.Func c C @@ -104,6 +107,7 @@ func Run[T comparable, PT interface { pass.ExportObjectFact(fn, PT(&v)) } } + return s, nil } diff --git a/lib/lifecycle/lifecycle.go b/lib/lifecycle/lifecycle.go index 5cc583b..d1184c6 100644 --- a/lib/lifecycle/lifecycle.go +++ b/lib/lifecycle/lifecycle.go @@ -32,14 +32,17 @@ func Funcs(pass *analysis.Pass, insp *inspector.Inspector) map[*ast.FuncDecl]str if !ok { return } + key, ok := kv.Key.(*ast.Ident) if !ok { return } + step, ok := steps[key.Name] if !ok { return } + var id *ast.Ident switch v := kv.Value.(type) { case *ast.Ident: @@ -49,6 +52,7 @@ func Funcs(pass *analysis.Pass, insp *inspector.Inspector) map[*ast.FuncDecl]str default: return } + fn, ok := pass.TypesInfo.Uses[id].(*types.Func) if !ok { return diff --git a/lib/nilguard/nilguard.go b/lib/nilguard/nilguard.go index 6034e20..d74e5c7 100644 --- a/lib/nilguard/nilguard.go +++ b/lib/nilguard/nilguard.go @@ -64,9 +64,11 @@ func ForEachFunc(pass *analysis.Pass, insp *inspector.Inspector, tests bool, vis body = fn.Body collectParams(fn.Type.Params) } + if body == nil { return } + ast.Inspect(body, func(x ast.Node) bool { if lit, ok := x.(*ast.FuncLit); ok { collectParams(lit.Type.Params) @@ -180,13 +182,11 @@ func guardedKey(pass *analysis.Pass, parents map[ast.Node]ast.Node, at ast.Node, // only evaluates once the left proved x non-nil; likewise `ok && *x` / `err != nil // || *x` for the companion of the call that produced x if p.Y == child { - if (p.Op == token.LAND && impliesNonNil(pass, p.X, key)) || - (p.Op == token.LOR && impliedByNil(pass, p.X, key)) { + if (p.Op == token.LAND && impliesNonNil(pass, p.X, key)) || (p.Op == token.LOR && impliedByNil(pass, p.X, key)) { return true } errKey, okKey := companionOf(pass, parents, at, key) - if (p.Op == token.LAND && condProvesValid(pass, p.X, errKey, okKey)) || - (p.Op == token.LOR && condProvesInvalid(pass, p.X, errKey, okKey)) { + if (p.Op == token.LAND && condProvesValid(pass, p.X, errKey, okKey)) || (p.Op == token.LOR && condProvesInvalid(pass, p.X, errKey, okKey)) { return true } } @@ -375,17 +375,20 @@ func equivalentKeys(pass *analysis.Pass, parents map[ast.Node]ast.Node, at ast.N if !ok { continue } + var rk string var rkOK bool if len(assign.Lhs) == len(assign.Rhs) { rk, rkOK = PathKey(pass, assign.Rhs[i]) } + if touches(lk, key) { if k := rk + strings.TrimPrefix(key, lk); rkOK && !stale(k) { keys = append(keys, k) } return false } + if k := lk + strings.TrimPrefix(key, rk); rkOK && touches(rk, key) && !stale(k) { keys = append(keys, k) } @@ -658,10 +661,12 @@ func literalField(expr ast.Expr, suffix string) ast.Expr { if u, ok := e.(*ast.UnaryExpr); ok && u.Op == token.AND { e = ast.Unparen(u.X) } + lit, ok := e.(*ast.CompositeLit) if !ok { return nil } + expr = nil for _, elt := range lit.Elts { kv, ok := elt.(*ast.KeyValueExpr) @@ -673,6 +678,7 @@ func literalField(expr ast.Expr, suffix string) ast.Expr { break } } + if expr == nil { return nil } @@ -688,17 +694,19 @@ func compositeIDArg(pass *analysis.Pass, expr ast.Expr, suffix string) ast.Expr if !ok || len(call.Args) < 2 { return nil } + fn := astx.CalledFunc(pass, call) - if fn == nil || fn.Pkg() == nil || fn.Pkg().Path() != commonIDsPkgPath || - (fn.Name() != "NewCompositeResourceID" && fn.Name() != "ParseCompositeResourceID") { + if fn == nil || fn.Pkg() == nil || fn.Pkg().Path() != commonIDsPkgPath || (fn.Name() != "NewCompositeResourceID" && fn.Name() != "ParseCompositeResourceID") { return nil } + switch suffix { case ".First": return call.Args[len(call.Args)-2] case ".Second": return call.Args[len(call.Args)-1] } + return nil } @@ -713,6 +721,7 @@ func companionKeys(pass *analysis.Pass, assign *ast.AssignStmt) (errKey, okKey s if !ok { continue } + obj := pass.TypesInfo.Uses[id] if obj == nil { obj = pass.TypesInfo.Defs[id] @@ -720,6 +729,7 @@ func companionKeys(pass *analysis.Pass, assign *ast.AssignStmt) (errKey, okKey s if obj == nil { continue } + if types.Identical(obj.Type(), errType) { errKey, _ = PathKey(pass, lhs) } else if basic, isBasic := obj.Type().Underlying().(*types.Basic); isBasic && basic.Kind() == types.Bool { @@ -913,14 +923,17 @@ func fromNonZero(pass *analysis.Pass, cmp *ast.BinaryExpr, key string) bool { if _, isBuiltin := pass.TypesInfo.Uses[id].(*types.Builtin); !isBuiltin { return false } + inner, ok := ast.Unparen(call.Args[0]).(*ast.CallExpr) if !ok || len(inner.Args) != 1 { return false } + fn := astx.CalledFunc(pass, inner) if fn == nil || fn.Pkg() == nil || fn.Pkg().Path() != pointerpkg.PkgPath || fn.Name() != "From" { return false } + k, ok := PathKey(pass, inner.Args[0]) return ok && k == key && (op == token.NEQ || op == token.GTR) && isZeroConst(pass, other) } diff --git a/lib/nilguard/returns.go b/lib/nilguard/returns.go index 3a2ef5c..042695d 100644 --- a/lib/nilguard/returns.go +++ b/lib/nilguard/returns.go @@ -60,6 +60,7 @@ func runReturns(pass *analysis.Pass) (any, error) { if decl.Type.Results == nil { return c, false } + for _, field := range decl.Type.Results.List { names := field.Names if len(names) == 0 { @@ -75,9 +76,11 @@ func runReturns(pass *analysis.Pass) (any, error) { } } } + if c.mask == 0 { return c, false } + c.parents = ParentMap(decl.Body) return c, true }, diff --git a/lib/pointerpkg/pointerpkg.go b/lib/pointerpkg/pointerpkg.go index d0a2ef1..394e7b6 100644 --- a/lib/pointerpkg/pointerpkg.go +++ b/lib/pointerpkg/pointerpkg.go @@ -56,6 +56,7 @@ func Ref(file *ast.File) (string, *analysis.TextEdit, bool) { if !ok { continue } + path := strings.Trim(imp.Path.Value, `"`) if !strings.Contains(strings.SplitN(path, "/", 2)[0], ".") { continue // standard library: a different section @@ -63,6 +64,7 @@ func Ref(file *ast.File) (string, *analysis.TextEdit, bool) { if imp.Name != nil && imp.Name.Name == "_" { continue // side-effect imports form their own section } + if path < PkgPath { // a trailing comment is not part of the spec's End; inserting between the two // would re-attach the comment (e.g. a nolint directive) to the new import @@ -78,6 +80,7 @@ func Ref(file *ast.File) (string, *analysis.TextEdit, bool) { if insertAfter.IsValid() { return PkgName, &analysis.TextEdit{Pos: insertAfter, End: insertAfter, NewText: []byte("\n\t" + newImport)}, true } + if insertBefore != nil { first := insertBefore.Pos() // keep a doc comment attached to the spec it documents rather than the new import @@ -92,6 +95,7 @@ func Ref(file *ast.File) (string, *analysis.TextEdit, bool) { if imp, ok := gen.Specs[len(gen.Specs)-1].(*ast.ImportSpec); ok && imp.Comment != nil { end = imp.Comment.End() } + return PkgName, &analysis.TextEdit{Pos: end, End: end, NewText: []byte("\n\n\t" + newImport)}, true } diff --git a/lib/requestbody/requestbody.go b/lib/requestbody/requestbody.go index b97da7c..25c0284 100644 --- a/lib/requestbody/requestbody.go +++ b/lib/requestbody/requestbody.go @@ -63,6 +63,7 @@ func CallBodyArg(pass *analysis.Pass, call *ast.CallExpr, arg int) bool { if arg < 0 || arg > 63 { return false } + f := facts.Of[bodyFact](pass, Analyzer).Get(astx.CalledFunc(pass, call)) if f.Serialised&(1<