Skip to content

Commit 2c59134

Browse files
calebdwcalebdw
authored
fix(unused-variables): treat get_defined_vars() as using all variables in scope (#158)
1 parent f7f7b0b commit 2c59134

3 files changed

Lines changed: 394 additions & 3 deletions

File tree

‎docs/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
6565
- **Analysis deadlock.** Lazily-parsed vendor files acquired two internal locks in the opposite order from the editor's file-change handler, causing a deadlock when both ran concurrently.
6666
- **External tool diagnostics on large files.** PHPStan, Mago, and PHPCS diagnostics no longer time out on files that produce a large report. Their output is now read while the tool is still running, so a report bigger than the operating system's pipe buffer can no longer stall the tool and force a timeout.
6767
- **Promote to constructor property.** Promoting a parameter whose property is declared together with others on one line (`private int $a, $b;`) no longer deletes the sibling properties. The action is now offered only when the property is declared on its own.
68+
- **`get_defined_vars()` counts as using every variable in scope.** A function or method that calls `get_defined_vars()` (for example to build a debug dump) no longer reports its local variables as unused, since the call reads all of them. Variables local to a nested closure or arrow function are still checked. Contributed by @calebdw in https://github.com/PHPantom-dev/phpantom_lsp/pull/158.
6869

6970
### Changed
7071

‎src/diagnostics/undefined_variables.rs‎

Lines changed: 309 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1214,6 +1214,315 @@ fn collect_compact_from_expr(expr: &Expression<'_>, vars: &mut HashSet<String>)
12141214
}
12151215
}
12161216

1217+
// ─── get_defined_vars() detection ───────────────────────────────────────────
1218+
1219+
/// Returns true if the statements contain a call to `get_defined_vars()`.
1220+
/// When present in a scope, all variables defined in that scope are
1221+
/// considered used (e.g. for debug dumps), so unused-variable diagnostics
1222+
/// should be suppressed for them.
1223+
pub(crate) fn has_get_defined_vars(statements: &[Statement<'_>]) -> bool {
1224+
for stmt in statements {
1225+
if stmt_has_get_defined_vars(stmt) {
1226+
return true;
1227+
}
1228+
}
1229+
false
1230+
}
1231+
1232+
fn stmt_has_get_defined_vars(stmt: &Statement<'_>) -> bool {
1233+
match stmt {
1234+
Statement::Expression(es) => expr_has_get_defined_vars(es.expression),
1235+
Statement::Return(ret) => ret.value.is_some_and(|v| expr_has_get_defined_vars(v)),
1236+
Statement::Echo(echo) => echo.values.iter().any(|v| expr_has_get_defined_vars(v)),
1237+
Statement::If(if_stmt) => {
1238+
if expr_has_get_defined_vars(if_stmt.condition) {
1239+
return true;
1240+
}
1241+
match &if_stmt.body {
1242+
IfBody::Statement(body) => {
1243+
if stmt_has_get_defined_vars(body.statement) {
1244+
return true;
1245+
}
1246+
for clause in body.else_if_clauses.iter() {
1247+
if expr_has_get_defined_vars(clause.condition)
1248+
|| stmt_has_get_defined_vars(clause.statement)
1249+
{
1250+
return true;
1251+
}
1252+
}
1253+
if let Some(ref el) = body.else_clause
1254+
&& stmt_has_get_defined_vars(el.statement)
1255+
{
1256+
return true;
1257+
}
1258+
}
1259+
IfBody::ColonDelimited(body) => {
1260+
for s in body.statements.iter() {
1261+
if stmt_has_get_defined_vars(s) {
1262+
return true;
1263+
}
1264+
}
1265+
for clause in body.else_if_clauses.iter() {
1266+
if expr_has_get_defined_vars(clause.condition) {
1267+
return true;
1268+
}
1269+
for s in clause.statements.iter() {
1270+
if stmt_has_get_defined_vars(s) {
1271+
return true;
1272+
}
1273+
}
1274+
}
1275+
if let Some(ref el) = body.else_clause {
1276+
for s in el.statements.iter() {
1277+
if stmt_has_get_defined_vars(s) {
1278+
return true;
1279+
}
1280+
}
1281+
}
1282+
}
1283+
}
1284+
false
1285+
}
1286+
Statement::Foreach(foreach) => {
1287+
expr_has_get_defined_vars(foreach.expression)
1288+
|| match &foreach.body {
1289+
ForeachBody::Statement(s) => stmt_has_get_defined_vars(s),
1290+
ForeachBody::ColonDelimited(b) => {
1291+
b.statements.iter().any(|s| stmt_has_get_defined_vars(s))
1292+
}
1293+
}
1294+
}
1295+
Statement::While(w) => {
1296+
expr_has_get_defined_vars(w.condition)
1297+
|| match &w.body {
1298+
WhileBody::Statement(s) => stmt_has_get_defined_vars(s),
1299+
WhileBody::ColonDelimited(b) => {
1300+
b.statements.iter().any(|s| stmt_has_get_defined_vars(s))
1301+
}
1302+
}
1303+
}
1304+
Statement::DoWhile(dw) => {
1305+
stmt_has_get_defined_vars(dw.statement) || expr_has_get_defined_vars(dw.condition)
1306+
}
1307+
Statement::For(for_stmt) => {
1308+
for_stmt
1309+
.initializations
1310+
.iter()
1311+
.any(|e| expr_has_get_defined_vars(e))
1312+
|| for_stmt
1313+
.conditions
1314+
.iter()
1315+
.any(|e| expr_has_get_defined_vars(e))
1316+
|| for_stmt
1317+
.increments
1318+
.iter()
1319+
.any(|e| expr_has_get_defined_vars(e))
1320+
|| match &for_stmt.body {
1321+
ForBody::Statement(s) => stmt_has_get_defined_vars(s),
1322+
ForBody::ColonDelimited(b) => {
1323+
b.statements.iter().any(|s| stmt_has_get_defined_vars(s))
1324+
}
1325+
}
1326+
}
1327+
Statement::Switch(sw) => {
1328+
expr_has_get_defined_vars(sw.expression)
1329+
|| sw.body.cases().iter().any(|c| match c {
1330+
SwitchCase::Expression(sc) => {
1331+
expr_has_get_defined_vars(sc.expression)
1332+
|| sc.statements.iter().any(|s| stmt_has_get_defined_vars(s))
1333+
}
1334+
SwitchCase::Default(dc) => {
1335+
dc.statements.iter().any(|s| stmt_has_get_defined_vars(s))
1336+
}
1337+
})
1338+
}
1339+
Statement::Try(try_stmt) => {
1340+
try_stmt
1341+
.block
1342+
.statements
1343+
.iter()
1344+
.any(|s| stmt_has_get_defined_vars(s))
1345+
|| try_stmt.catch_clauses.iter().any(|c| {
1346+
c.block
1347+
.statements
1348+
.iter()
1349+
.any(|s| stmt_has_get_defined_vars(s))
1350+
})
1351+
|| try_stmt.finally_clause.as_ref().is_some_and(|f| {
1352+
f.block
1353+
.statements
1354+
.iter()
1355+
.any(|s| stmt_has_get_defined_vars(s))
1356+
})
1357+
}
1358+
Statement::Block(block) => block
1359+
.statements
1360+
.iter()
1361+
.any(|s| stmt_has_get_defined_vars(s)),
1362+
_ => false,
1363+
}
1364+
}
1365+
1366+
fn expr_has_get_defined_vars(expr: &Expression<'_>) -> bool {
1367+
match expr {
1368+
Expression::Call(Call::Function(fc)) => {
1369+
if let Expression::Identifier(ident) = fc.function
1370+
&& ident.value().eq_ignore_ascii_case(b"get_defined_vars")
1371+
{
1372+
return true;
1373+
}
1374+
expr_has_get_defined_vars(fc.function)
1375+
// Recurse into arguments for nested calls.
1376+
|| fc
1377+
.argument_list
1378+
.arguments
1379+
.iter()
1380+
.any(|a| expr_has_get_defined_vars(a.value()))
1381+
}
1382+
Expression::Call(Call::Method(mc)) => {
1383+
expr_has_get_defined_vars(mc.object)
1384+
|| mc
1385+
.argument_list
1386+
.arguments
1387+
.iter()
1388+
.any(|a| expr_has_get_defined_vars(a.value()))
1389+
}
1390+
Expression::Call(Call::NullSafeMethod(mc)) => {
1391+
expr_has_get_defined_vars(mc.object)
1392+
|| mc
1393+
.argument_list
1394+
.arguments
1395+
.iter()
1396+
.any(|a| expr_has_get_defined_vars(a.value()))
1397+
}
1398+
Expression::Call(Call::StaticMethod(sc)) => {
1399+
expr_has_get_defined_vars(sc.class)
1400+
|| sc
1401+
.argument_list
1402+
.arguments
1403+
.iter()
1404+
.any(|a| expr_has_get_defined_vars(a.value()))
1405+
}
1406+
Expression::Access(access) => match access {
1407+
Access::Property(pa) => expr_has_get_defined_vars(pa.object),
1408+
Access::NullSafeProperty(pa) => expr_has_get_defined_vars(pa.object),
1409+
Access::StaticProperty(spa) => expr_has_get_defined_vars(spa.class),
1410+
Access::ClassConstant(cca) => expr_has_get_defined_vars(cca.class),
1411+
},
1412+
Expression::ArrayAccess(aa) => {
1413+
expr_has_get_defined_vars(aa.array) || expr_has_get_defined_vars(aa.index)
1414+
}
1415+
Expression::ArrayAppend(append) => expr_has_get_defined_vars(append.array),
1416+
Expression::Array(arr) => arr
1417+
.elements
1418+
.iter()
1419+
.any(|e| array_elem_has_get_defined_vars(e)),
1420+
Expression::LegacyArray(arr) => arr
1421+
.elements
1422+
.iter()
1423+
.any(|e| array_elem_has_get_defined_vars(e)),
1424+
Expression::List(list) => list
1425+
.elements
1426+
.iter()
1427+
.any(|e| array_elem_has_get_defined_vars(e)),
1428+
Expression::Assignment(a) => {
1429+
expr_has_get_defined_vars(a.lhs) || expr_has_get_defined_vars(a.rhs)
1430+
}
1431+
Expression::Binary(b) => {
1432+
expr_has_get_defined_vars(b.lhs) || expr_has_get_defined_vars(b.rhs)
1433+
}
1434+
Expression::UnaryPrefix(u) => expr_has_get_defined_vars(u.operand),
1435+
Expression::UnaryPostfix(u) => expr_has_get_defined_vars(u.operand),
1436+
Expression::Parenthesized(p) => expr_has_get_defined_vars(p.expression),
1437+
Expression::Conditional(c) => {
1438+
expr_has_get_defined_vars(c.condition)
1439+
|| c.then.is_some_and(|t| expr_has_get_defined_vars(t))
1440+
|| expr_has_get_defined_vars(c.r#else)
1441+
}
1442+
Expression::Instantiation(inst) => {
1443+
expr_has_get_defined_vars(inst.class)
1444+
|| inst.argument_list.as_ref().is_some_and(|al| {
1445+
al.arguments
1446+
.iter()
1447+
.any(|a| expr_has_get_defined_vars(a.value()))
1448+
})
1449+
}
1450+
Expression::Throw(t) => expr_has_get_defined_vars(t.exception),
1451+
Expression::Clone(c) => expr_has_get_defined_vars(c.object),
1452+
Expression::Yield(yield_expr) => match yield_expr {
1453+
Yield::Value(yv) => yv.value.is_some_and(expr_has_get_defined_vars),
1454+
Yield::Pair(yp) => {
1455+
expr_has_get_defined_vars(yp.key) || expr_has_get_defined_vars(yp.value)
1456+
}
1457+
Yield::From(yf) => expr_has_get_defined_vars(yf.iterator),
1458+
},
1459+
Expression::Match(m) => {
1460+
expr_has_get_defined_vars(m.expression)
1461+
|| m.arms.iter().any(|arm| match arm {
1462+
MatchArm::Expression(ea) => {
1463+
ea.conditions.iter().any(|c| expr_has_get_defined_vars(c))
1464+
|| expr_has_get_defined_vars(ea.expression)
1465+
}
1466+
MatchArm::Default(da) => expr_has_get_defined_vars(da.expression),
1467+
})
1468+
}
1469+
Expression::Construct(construct) => match construct {
1470+
Construct::Isset(isset) => isset.values.iter().any(|v| expr_has_get_defined_vars(v)),
1471+
Construct::Empty(empty) => expr_has_get_defined_vars(empty.value),
1472+
Construct::Eval(eval) => expr_has_get_defined_vars(eval.value),
1473+
Construct::Include(inc) => expr_has_get_defined_vars(inc.value),
1474+
Construct::IncludeOnce(inc) => expr_has_get_defined_vars(inc.value),
1475+
Construct::Require(req) => expr_has_get_defined_vars(req.value),
1476+
Construct::RequireOnce(req) => expr_has_get_defined_vars(req.value),
1477+
Construct::Print(print) => expr_has_get_defined_vars(print.value),
1478+
Construct::Exit(exit) => exit.arguments.as_ref().is_some_and(|args| {
1479+
args.arguments
1480+
.iter()
1481+
.any(|a| expr_has_get_defined_vars(a.value()))
1482+
}),
1483+
Construct::Die(die) => die.arguments.as_ref().is_some_and(|args| {
1484+
args.arguments
1485+
.iter()
1486+
.any(|a| expr_has_get_defined_vars(a.value()))
1487+
}),
1488+
},
1489+
Expression::CompositeString(composite) => composite.parts().iter().any(|part| match part {
1490+
StringPart::Expression(inner_expr) => expr_has_get_defined_vars(inner_expr),
1491+
StringPart::BracedExpression(braced) => expr_has_get_defined_vars(braced.expression),
1492+
StringPart::Literal(_) => false,
1493+
}),
1494+
Expression::Pipe(pipe) => {
1495+
expr_has_get_defined_vars(pipe.input) || expr_has_get_defined_vars(pipe.callable)
1496+
}
1497+
Expression::PartialApplication(partial) => match partial {
1498+
PartialApplication::Function(func_pa) => expr_has_get_defined_vars(func_pa.function),
1499+
PartialApplication::Method(method_pa) => expr_has_get_defined_vars(method_pa.object),
1500+
PartialApplication::StaticMethod(static_pa) => {
1501+
expr_has_get_defined_vars(static_pa.class)
1502+
}
1503+
},
1504+
Expression::AnonymousClass(anon) => anon.argument_list.as_ref().is_some_and(|args| {
1505+
args.arguments
1506+
.iter()
1507+
.any(|a| expr_has_get_defined_vars(a.value()))
1508+
}),
1509+
// Don't recurse into closures/arrow functions.
1510+
Expression::Closure(_) | Expression::ArrowFunction(_) => false,
1511+
_ => false,
1512+
}
1513+
}
1514+
1515+
fn array_elem_has_get_defined_vars(elem: &ArrayElement<'_>) -> bool {
1516+
match elem {
1517+
ArrayElement::KeyValue(kv) => {
1518+
expr_has_get_defined_vars(kv.key) || expr_has_get_defined_vars(kv.value)
1519+
}
1520+
ArrayElement::Value(v) => expr_has_get_defined_vars(v.value),
1521+
ArrayElement::Variadic(s) => expr_has_get_defined_vars(s.value),
1522+
ArrayElement::Missing(_) => false,
1523+
}
1524+
}
1525+
12171526
// ─── @var annotation collection ─────────────────────────────────────────────
12181527

12191528
/// Scan the source text for `/** @var Type $varName */` inline

0 commit comments

Comments
 (0)