From 5c4c11fb0fd6dd778f65d45763b8d776e8073608 Mon Sep 17 00:00:00 2001 From: Richardas Kuchinskas Date: Tue, 5 Mar 2024 17:53:30 +0300 Subject: [PATCH] added new warning and QuickFix for it: lack of declare(strict_types=1) section new rules for .gitignore new tests refactored warnings --- .gitignore | 1 + src/ir/irutil/keyword.go | 2 ++ src/linter/block_linter.go | 7 ++++- src/linter/quickfix.go | 8 ++++++ src/linter/report.go | 27 ++++++++++++------- src/linter/root.go | 15 ++++++++--- src/linter/worker.go | 5 ++++ .../quickfix/arraySyntax.php.fix.expected | 1 + .../quickfix/binaryOp.php.fix.expected | 1 + .../quickfix/callSimplify.php.fix.expected | 1 + .../quickfix/complexExamples.php.fix.expected | 1 + .../quickfix/constCase.php.fix.expected | 1 + .../testdata/quickfix/declareStrictTypes.php | 2 +- .../declareStrictTypes.php.fix.expected | 2 +- .../quickfix/getTypeMisUse.php.fix.expected | 1 + .../quickfix/indexingSyntax.php.fix.expected | 1 + .../noDeclareStrictTypesSection_1.php | 4 +++ ...clareStrictTypesSection_1.php.fix.expected | 5 ++++ .../noDeclareStrictTypesSection_2.php | 5 ++++ ...clareStrictTypesSection_2.php.fix.expected | 6 +++++ .../quickfix/propNullDefault.php.fix.expected | 1 + .../quickfix/ternarySimplify.php.fix.expected | 1 + .../quickfix/trailingComma.php.fix.expected | 1 + .../quickfix/unaryRepeat.php.fix.expected | 1 + 24 files changed, 84 insertions(+), 16 deletions(-) create mode 100644 src/tests/golden/testdata/quickfix/noDeclareStrictTypesSection_1.php create mode 100644 src/tests/golden/testdata/quickfix/noDeclareStrictTypesSection_1.php.fix.expected create mode 100644 src/tests/golden/testdata/quickfix/noDeclareStrictTypesSection_2.php create mode 100644 src/tests/golden/testdata/quickfix/noDeclareStrictTypesSection_2.php.fix.expected diff --git a/.gitignore b/.gitignore index b9445adcc..23e5ca743 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ y.output .idea vendor dev +src/tests/golden/testdata/quickfix/*.fix \ No newline at end of file diff --git a/src/ir/irutil/keyword.go b/src/ir/irutil/keyword.go index 764104929..8cf454865 100644 --- a/src/ir/irutil/keyword.go +++ b/src/ir/irutil/keyword.go @@ -14,6 +14,8 @@ func Keywords(n ir.Node) []*token.Token { switch n := n.(type) { case *ir.FunctionStmt: return []*token.Token{n.FunctionTkn} + case *ir.DeclareStmt: + return []*token.Token{n.DeclareTkn} case *ir.DefaultStmt: return []*token.Token{n.DefaultTkn} case *ir.CaseStmt: diff --git a/src/linter/block_linter.go b/src/linter/block_linter.go index 1fa947d83..6b163b8ce 100644 --- a/src/linter/block_linter.go +++ b/src/linter/block_linter.go @@ -1537,6 +1537,10 @@ func (b *blockLinter) metaInfo() *meta.Info { } func (b *blockLinter) checkDeclareStrictTypes(statement *ir.DeclareStmt) { + if b.walker.r.strictTypes { + return + } + for _, i := range statement.Consts { node, ok := i.(*ir.ConstantStmt) if !ok { @@ -1546,6 +1550,7 @@ func (b *blockLinter) checkDeclareStrictTypes(statement *ir.DeclareStmt) { if node.ConstantName.Value != "strict_types" { return } + value, isNumber := node.Expr.(*ir.Lnumber) if !isNumber { return @@ -1555,7 +1560,7 @@ func (b *blockLinter) checkDeclareStrictTypes(statement *ir.DeclareStmt) { return } - b.report(statement, LevelError, "notStrictTypes", "strict_types value is not 1") + b.report(statement, LevelWarning, "notStrictTypes", "strict_types value is not 1") b.walker.r.addQuickFix("notStrictTypes", b.quickfix.StrictTypes(value)) } } diff --git a/src/linter/quickfix.go b/src/linter/quickfix.go index 36f7cc4e2..50d6d6204 100644 --- a/src/linter/quickfix.go +++ b/src/linter/quickfix.go @@ -42,6 +42,14 @@ func (g *QuickFixGenerator) StrictTypes(lnumber *ir.Lnumber) quickfix.TextEdit { } } +func (g *QuickFixGenerator) CreateDeclareStrictTypes(root *ir.Root) quickfix.TextEdit { + return quickfix.TextEdit{ + StartPos: root.Position.StartPos, + EndPos: root.Position.StartPos, + Replacement: "declare(strict_types=1);\n", + } +} + func (g *QuickFixGenerator) NullForNotNullableProperty(prop *ir.PropertyStmt) quickfix.TextEdit { from := prop.Position.StartPos to := prop.Variable.Position.EndPos diff --git a/src/linter/report.go b/src/linter/report.go index faf624d04..a9e831257 100644 --- a/src/linter/report.go +++ b/src/linter/report.go @@ -34,6 +34,15 @@ func addBuiltinCheckers(reg *CheckersRegistry) { After: `declare(strict_types=1);`, }, + { + Name: "noDeclareSection", + Default: true, + Quickfix: true, + Comment: "Report declare(strict_types=1) has not been set.", + Before: ` `, + After: `declare(strict_types=1);`, + }, + { Name: "emptyStmt", Default: true, @@ -1194,8 +1203,8 @@ func DiffReports(gitRepo string, diffArgs []string, changesList []git.Change, ch } } - old := reportListToMap(oldList) - new := reportListToMap(newList) + oldReportMap := reportListToMap(oldList) + newReportMap := reportListToMap(newList) changes := gitChangesToMap(changesList) var mu sync.Mutex @@ -1205,7 +1214,7 @@ func DiffReports(gitRepo string, diffArgs []string, changesList []git.Change, ch limitCh := make(chan struct{}, maxConcurrency) - for filename, list := range new { + for filename, list := range newReportMap { wg.Add(1) go func(filename string, list []*Report) { limitCh <- struct{}{} @@ -1221,7 +1230,7 @@ func DiffReports(gitRepo string, diffArgs []string, changesList []git.Change, ch oldName = filename // full diff mode } - reports, err := diffReportsList(gitRepo, ignoreCommits, diffArgs, filename, c, old[oldName], list) + reports, err := diffReportsList(gitRepo, ignoreCommits, diffArgs, filename, c, oldReportMap[oldName], list) if err != nil { mu.Lock() resErr = err @@ -1275,8 +1284,8 @@ func diffReportsList(gitRepo string, ignoreCommits map[string]struct{}, diffArgs } } - old, oldMaxLine := reportListToPerLineMap(oldList) - new, newMaxLine := reportListToPerLineMap(newList) + oldPerLineMap, oldMaxLine := reportListToPerLineMap(oldList) + newPerLineMap, newMaxLine := reportListToPerLineMap(newList) var maxLine = oldMaxLine if newMaxLine > maxLine { @@ -1293,17 +1302,17 @@ func diffReportsList(gitRepo string, ignoreCommits map[string]struct{}, diffArgs // just deletion if ok && ch.new.HaveRange && ch.new.Range == 0 { oldLine = ch.old.To - newLine-- // cancel the increment of newLine, because code was deleted, no new lines added + newLine-- // cancel the increment of newLine, because code was deleted, no newPerLineMap lines added continue } - res = maybeAppendReports(res, new, old, newLine, oldLine, blame, ignoreCommits) + res = maybeAppendReports(res, newPerLineMap, oldPerLineMap, newLine, oldLine, blame, ignoreCommits) if ok { oldLine = 0 // all changes and additions must be checked for j := newLine + 1; j <= ch.new.To; j++ { newLine = j - res = maybeAppendReports(res, new, old, newLine, oldLine, blame, ignoreCommits) + res = maybeAppendReports(res, newPerLineMap, oldPerLineMap, newLine, oldLine, blame, ignoreCommits) } oldLine = ch.old.To } diff --git a/src/linter/root.go b/src/linter/root.go index feafef310..b6cd089a8 100644 --- a/src/linter/root.go +++ b/src/linter/root.go @@ -69,10 +69,12 @@ type rootWalker struct { // name matches the pattern and @linter disable was encountered // strictTypes is true if file contains `declare(strict_types=1)`. - strictTypes bool - strictMixed bool + strictTypes bool + strictMixed bool + declareSection bool - reports []*Report + reports []*Report + quickfix *QuickFixGenerator config *Config @@ -141,7 +143,12 @@ func (d *rootWalker) EnterNode(n ir.Node) (res bool) { } if c.ConstantName.Value == "strict_types" { v, ok := c.Expr.(*ir.Lnumber) - if ok && v.Value == "1" { + if !ok { + continue + } + + d.declareSection = true + if v.Value == "1" { d.strictTypes = true } } diff --git a/src/linter/worker.go b/src/linter/worker.go index 6cfa8c5a0..382f3c01b 100644 --- a/src/linter/worker.go +++ b/src/linter/worker.go @@ -315,6 +315,11 @@ func (w *Worker) analyzeFile(file *workspace.File, rootNode *ir.Root) (*rootWalk } walker.afterLeaveFile() + if !walker.declareSection { + walker.Report(rootNode, LevelWarning, "noDeclareSection", "You have no declare section with strict_types=1 argument") + walker.addQuickFix("notStrictTypes", walker.quickfix.CreateDeclareStrictTypes(rootNode)) + } + if len(walker.ctx.fixes) != 0 { needApplyFixes := !file.AutoGenerated() || w.config.CheckAutoGenerated diff --git a/src/tests/golden/testdata/quickfix/arraySyntax.php.fix.expected b/src/tests/golden/testdata/quickfix/arraySyntax.php.fix.expected index 4e4199df7..5a0d176c5 100644 --- a/src/tests/golden/testdata/quickfix/arraySyntax.php.fix.expected +++ b/src/tests/golden/testdata/quickfix/arraySyntax.php.fix.expected @@ -1,5 +1,6 @@ 1, 'b' => 2, 'c' => 3]; if (array_key_exists('z', $array)) { // 2 diff --git a/src/tests/golden/testdata/quickfix/complexExamples.php.fix.expected b/src/tests/golden/testdata/quickfix/complexExamples.php.fix.expected index 503b05611..15e87f4c4 100644 --- a/src/tests/golden/testdata/quickfix/complexExamples.php.fix.expected +++ b/src/tests/golden/testdata/quickfix/complexExamples.php.fix.expected @@ -1,5 +1,6 @@