Skip to content

Commit

Permalink
added new warning and QuickFix for it: lack of declare(strict_types=1…
Browse files Browse the repository at this point in the history
…) section

new rules for .gitignore
new tests
refactored warnings
  • Loading branch information
Hidanio committed Mar 5, 2024
1 parent 4fa5cf4 commit 5c4c11f
Show file tree
Hide file tree
Showing 24 changed files with 84 additions and 16 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ y.output
.idea
vendor
dev
src/tests/golden/testdata/quickfix/*.fix
2 changes: 2 additions & 0 deletions src/ir/irutil/keyword.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
7 changes: 6 additions & 1 deletion src/linter/block_linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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))
}
}
8 changes: 8 additions & 0 deletions src/linter/quickfix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 18 additions & 9 deletions src/linter/report.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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{}{}
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
Expand Down
15 changes: 11 additions & 4 deletions src/linter/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
}
}
Expand Down
5 changes: 5 additions & 0 deletions src/linter/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php

declare(strict_types=1);
class Foo {}

$_ = [1,2,3];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php

declare(strict_types=1);
function binaryOp() {
$x += $y;
$x -= $y;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php

declare(strict_types=1);
$array = ['a' => 1, 'b' => 2, 'c' => 3];

if (array_key_exists('z', $array)) { // 2
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php

declare(strict_types=1);
function foo(array $a) { return 0; }

$_ = [isset($x) ? $x : $y]; // expressions in global scope
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php

declare(strict_types=1);
function f($a) {
if ($a === null) {
return false;
Expand Down
2 changes: 1 addition & 1 deletion src/tests/golden/testdata/quickfix/declareStrictTypes.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?php
declare(strict_types = 0);
declare(strict_types=0);

function test():string {
return "test";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?php
declare(strict_types = 1);
declare(strict_types=1);

function test():string {
return "test";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php

declare(strict_types=1);
function getTypeMisUse(mixed $var) {
if (is_string($var)) {
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php

declare(strict_types=1);
$b = [];
$_ = $b[0];
$_ = $b[0][0];
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<?php
function test():string {
return "test";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?php
declare(strict_types=1);
function test():string {
return "test";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?php
declare(ticks=1);
function test():string {
return "test";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?php
declare(strict_types=1);
declare(ticks=1);
function test():string {
return "test";
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php

declare(strict_types=1);
class Boo {}
class Zoo {}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php

declare(strict_types=1);
$a = 5;
function f(): int {
global $a;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php

declare(strict_types=1);
function f() {
$_ = [10, 20, 30]; // ok

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php

declare(strict_types=1);
function unaryRepeat() {
$a = 100;

Expand Down

0 comments on commit 5c4c11f

Please sign in to comment.