Skip to content

Commit c17f6ee

Browse files
google-labs-jules[bot]google-labs-jules[bot]
committed
Add Laravel Catch Block Rule for PHP
- Implemented `LaravelCatchBlockRule` in `analyzers/php/laravel_catch.go` using AST parsing. - Added dependency `github.com/z7zmey/php-parser` for robust PHP parsing. - Updated `PHPAnalyzer` in `analyzers/php/php.go` to include the new rule and process its findings. - The rule detects missing `report()` calls in catch blocks (Critical) and `report()` calls that are not the first statement (Medium) in Laravel app files. - Added comprehensive tests in `analyzers/php/laravel_catch_test.go`.
1 parent f1dd3a6 commit c17f6ee

5 files changed

Lines changed: 355 additions & 28 deletions

File tree

‎analyzers/php/laravel_catch.go‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
package php
2+
3+
import (
4+
"code-analyzer/models"
5+
6+
"github.com/z7zmey/php-parser/node"
7+
"github.com/z7zmey/php-parser/node/expr"
8+
"github.com/z7zmey/php-parser/node/name"
9+
"github.com/z7zmey/php-parser/node/stmt"
10+
"github.com/z7zmey/php-parser/php7"
11+
"github.com/z7zmey/php-parser/walker"
12+
)
13+
14+
// LaravelCatchBlockRule checks for proper error reporting in try-catch blocks
15+
type LaravelCatchBlockRule struct{}
16+
17+
func (r *LaravelCatchBlockRule) Name() string {
18+
return "Laravel Catch Block Rule"
19+
}
20+
21+
// LaravelCatchBlockFinding holds the issues found by the rule
22+
type LaravelCatchBlockFinding struct {
23+
Issues []models.Issue
24+
}
25+
26+
func (r *LaravelCatchBlockRule) Apply(content string) interface{} {
27+
parser := php7.NewParser([]byte(content), "7.4")
28+
parser.Parse()
29+
30+
root := parser.GetRootNode()
31+
if root == nil {
32+
return nil
33+
}
34+
35+
v := &catchVisitor{
36+
issues: []models.Issue{},
37+
}
38+
root.Walk(v)
39+
40+
if len(v.issues) == 0 {
41+
return nil
42+
}
43+
44+
return LaravelCatchBlockFinding{
45+
Issues: v.issues,
46+
}
47+
}
48+
49+
type catchVisitor struct {
50+
issues []models.Issue
51+
}
52+
53+
// Ensure catchVisitor implements walker.Visitor
54+
var _ walker.Visitor = (*catchVisitor)(nil)
55+
56+
func (v *catchVisitor) EnterNode(w walker.Walkable) bool {
57+
if n, ok := w.(node.Node); ok {
58+
if catchNode, ok := n.(*stmt.Catch); ok {
59+
v.analyzeCatch(catchNode)
60+
}
61+
}
62+
return true
63+
}
64+
65+
func (v *catchVisitor) LeaveNode(w walker.Walkable) {
66+
// no-op
67+
}
68+
69+
func (v *catchVisitor) EnterChildNode(key string, w walker.Walkable) {
70+
// no-op
71+
}
72+
73+
func (v *catchVisitor) LeaveChildNode(key string, w walker.Walkable) {
74+
// no-op
75+
}
76+
77+
func (v *catchVisitor) EnterChildList(key string, w walker.Walkable) {
78+
// no-op
79+
}
80+
81+
func (v *catchVisitor) LeaveChildList(key string, w walker.Walkable) {
82+
// no-op
83+
}
84+
85+
func (v *catchVisitor) analyzeCatch(n *stmt.Catch) {
86+
// Check statements in the catch block
87+
stmts := n.Stmts
88+
89+
foundReport := false
90+
isFirst := false
91+
92+
for i, s := range stmts {
93+
// Look for report(...) call
94+
if isReportCall(s) {
95+
foundReport = true
96+
if i == 0 {
97+
isFirst = true
98+
}
99+
break
100+
}
101+
}
102+
103+
// Default line number 0 if position not available, but usually it is.
104+
startLine := 0
105+
if n.GetPosition() != nil {
106+
startLine = n.GetPosition().StartLine
107+
}
108+
109+
if !foundReport {
110+
v.issues = append(v.issues, models.Issue{
111+
Description: "Critical: Catch block missing report() call in Laravel app file",
112+
Line: startLine,
113+
Severity: "critical",
114+
})
115+
} else if !isFirst {
116+
v.issues = append(v.issues, models.Issue{
117+
Description: "Medium Risk: report() call is not the first statement in catch block",
118+
Line: startLine,
119+
Severity: "medium",
120+
})
121+
}
122+
}
123+
124+
func isReportCall(n node.Node) bool {
125+
// We expect an expression statement containing a function call
126+
if exprStmt, ok := n.(*stmt.Expression); ok {
127+
if funcCall, ok := exprStmt.Expr.(*expr.FunctionCall); ok {
128+
// Check function name
129+
if nameNode, ok := funcCall.Function.(*name.Name); ok {
130+
// name.Name parts are parts of the namespace/name
131+
// For "report", it should be a single part "report"
132+
parts := nameNode.Parts
133+
if len(parts) == 1 {
134+
if s, ok := parts[0].(*name.NamePart); ok {
135+
return s.Value == "report"
136+
}
137+
}
138+
}
139+
}
140+
}
141+
return false
142+
}

‎analyzers/php/laravel_catch_test.go‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
package php
2+
3+
import (
4+
"testing"
5+
)
6+
7+
func TestLaravelCatchBlockRule_Apply(t *testing.T) {
8+
rule := &LaravelCatchBlockRule{}
9+
10+
tests := []struct {
11+
name string
12+
content string
13+
wantIssues int
14+
wantSeverity string
15+
wantLine int
16+
}{
17+
{
18+
name: "Critical: No report call",
19+
content: `<?php
20+
namespace App\Http\Controllers;
21+
22+
class TestController {
23+
public function index() {
24+
try {
25+
// something
26+
} catch (\Exception $e) {
27+
// silent fail
28+
}
29+
}
30+
}
31+
`,
32+
wantIssues: 1,
33+
wantSeverity: "critical",
34+
wantLine: 8,
35+
},
36+
{
37+
name: "Medium: report call not first",
38+
content: `<?php
39+
namespace App\Http\Controllers;
40+
41+
class TestController {
42+
public function index() {
43+
try {
44+
// something
45+
} catch (\Exception $e) {
46+
\Log::error($e);
47+
report($e);
48+
}
49+
}
50+
}
51+
`,
52+
wantIssues: 1,
53+
wantSeverity: "medium",
54+
wantLine: 8,
55+
},
56+
{
57+
name: "Valid: report call is first",
58+
content: `<?php
59+
namespace App\Http\Controllers;
60+
61+
class TestController {
62+
public function index() {
63+
try {
64+
// something
65+
} catch (\Exception $e) {
66+
report($e);
67+
return response()->json(['error' => 'fail']);
68+
}
69+
}
70+
}
71+
`,
72+
wantIssues: 0,
73+
},
74+
{
75+
name: "Multiple catch blocks",
76+
content: `<?php
77+
class Test {
78+
function test() {
79+
try {}
80+
catch (A $e) { report($e); }
81+
catch (B $e) {}
82+
}
83+
}
84+
`,
85+
wantIssues: 1,
86+
wantSeverity: "critical",
87+
wantLine: 6, // The line of catch (B $e)
88+
},
89+
}
90+
91+
for _, tt := range tests {
92+
t.Run(tt.name, func(t *testing.T) {
93+
result := rule.Apply(tt.content)
94+
95+
if tt.wantIssues == 0 {
96+
if result != nil {
97+
t.Errorf("Expected nil result, got %v", result)
98+
}
99+
return
100+
}
101+
102+
if result == nil {
103+
t.Errorf("Expected issues, got nil")
104+
return
105+
}
106+
107+
finding, ok := result.(LaravelCatchBlockFinding)
108+
if !ok {
109+
t.Errorf("Expected LaravelCatchBlockFinding, got %T", result)
110+
return
111+
}
112+
113+
if len(finding.Issues) != tt.wantIssues {
114+
t.Errorf("Expected %d issues, got %d", tt.wantIssues, len(finding.Issues))
115+
}
116+
117+
if len(finding.Issues) > 0 {
118+
issue := finding.Issues[0]
119+
if issue.Severity != tt.wantSeverity {
120+
t.Errorf("Expected severity %s, got %s", tt.wantSeverity, issue.Severity)
121+
}
122+
if issue.Line != tt.wantLine {
123+
t.Errorf("Expected line %d, got %d", tt.wantLine, issue.Line)
124+
}
125+
}
126+
})
127+
}
128+
}

‎analyzers/php/php.go‎

Lines changed: 54 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ func NewPHPAnalyzer() *PHPAnalyzer {
2323
return &PHPAnalyzer{
2424
rules: []analyzers.Rule{
2525
&CommentedFunctionsRule{},
26+
&LaravelCatchBlockRule{},
2627
},
2728
}
2829
}
@@ -57,10 +58,11 @@ func (a *PHPAnalyzer) Run(config analyzers.Config) ([]models.Issue, error) {
5758

5859
analysis := a.analyzeFile(path)
5960
if analysis != nil {
60-
if analysis.CommentedFunctions < config.MinValue {
61+
// Skip if below threshold AND no other issues
62+
if analysis.CommentedFunctions < config.MinValue && len(analysis.Issues) == 0 {
6163
return nil
6264
}
63-
if config.MinRatio > 0 && analysis.CommentRatio < config.MinRatio {
65+
if config.MinRatio > 0 && analysis.CommentRatio < config.MinRatio && len(analysis.Issues) == 0 {
6466
return nil
6567
}
6668

@@ -111,43 +113,67 @@ func (a *PHPAnalyzer) analyzeFile(path string) *models.PHPFileAnalysis {
111113
if err != nil {
112114
return nil
113115
}
116+
contentStr := string(content)
117+
118+
var analysis *models.PHPFileAnalysis
119+
var allIssues []models.Issue
114120

115121
// Apply commented functions rule
116-
rule := &CommentedFunctionsRule{}
117-
finding := rule.Apply(string(content))
122+
cfRule := &CommentedFunctionsRule{}
123+
if finding := cfRule.Apply(contentStr); finding != nil {
124+
result := finding.(CommentedFunctionsFinding)
125+
126+
totalBytes := len(content)
127+
commentedBytes := len(result.CommentedList) * 20 // rough estimate
128+
ratio := 0.0
129+
if len(result.AllFunctions) > 0 {
130+
ratio = float64(len(result.CommentedList)) / float64(len(result.AllFunctions)) * 100
131+
}
118132

119-
if finding == nil {
120-
return nil
133+
// Set path for issues
134+
for i := range result.Issues {
135+
result.Issues[i].Path = path
136+
}
137+
allIssues = append(allIssues, result.Issues...)
138+
139+
analysis = &models.PHPFileAnalysis{
140+
Path: path,
141+
TotalFunctions: len(result.AllFunctions),
142+
CommentedFunctions: len(result.CommentedList),
143+
FunctionList: result.AllFunctions,
144+
CommentedList: result.CommentedList,
145+
CommentRatio: ratio,
146+
TotalBytes: totalBytes,
147+
CommentedBytes: commentedBytes,
148+
}
121149
}
122150

123-
result := finding.(CommentedFunctionsFinding)
124-
if len(result.CommentedList) == 0 {
125-
return nil
151+
// Apply Laravel Catch Block Rule
152+
if strings.Contains(path, "app/") {
153+
lcbRule := &LaravelCatchBlockRule{}
154+
if finding := lcbRule.Apply(contentStr); finding != nil {
155+
result := finding.(LaravelCatchBlockFinding)
156+
for i := range result.Issues {
157+
result.Issues[i].Path = path
158+
}
159+
allIssues = append(allIssues, result.Issues...)
160+
}
126161
}
127162

128-
// Set path for issues
129-
for i := range result.Issues {
130-
result.Issues[i].Path = path
163+
if analysis == nil && len(allIssues) == 0 {
164+
return nil
131165
}
132166

133-
totalBytes := len(content)
134-
commentedBytes := len(result.CommentedList) * 20 // rough estimate
135-
ratio := 0.0
136-
if len(result.AllFunctions) > 0 {
137-
ratio = float64(len(result.CommentedList)) / float64(len(result.AllFunctions)) * 100
167+
if analysis == nil {
168+
// Create a basic analysis object if we only have other issues
169+
analysis = &models.PHPFileAnalysis{
170+
Path: path,
171+
TotalBytes: len(content),
172+
}
138173
}
139174

140-
return &models.PHPFileAnalysis{
141-
Path: path,
142-
TotalFunctions: len(result.AllFunctions),
143-
CommentedFunctions: len(result.CommentedList),
144-
FunctionList: result.AllFunctions,
145-
CommentedList: result.CommentedList,
146-
CommentRatio: ratio,
147-
TotalBytes: totalBytes,
148-
CommentedBytes: commentedBytes,
149-
Issues: result.Issues,
150-
}
175+
analysis.Issues = allIssues
176+
return analysis
151177
}
152178

153179
func (a *PHPAnalyzer) printResults(results []models.PHPFileAnalysis, totalFunctions, totalCommented int) {

‎go.mod‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,9 @@ module code-analyzer
33
go 1.24
44

55
require gopkg.in/yaml.v3 v3.0.1
6+
7+
require (
8+
github.com/pkg/profile v1.4.0 // indirect
9+
github.com/yookoala/realpath v1.0.0 // indirect
10+
github.com/z7zmey/php-parser v0.7.2 // indirect
11+
)

0 commit comments

Comments
 (0)