Skip to content

Commit 8722f77

Browse files
martindemellometa-codesync[bot]
authored andcommitted
Treat method calls on builtin literals as side-effect-free
Summary: A method call whose receiver is a freshly-constructed builtin literal, e.g. `[].append(x)`, operates on a value with no external aliasing, so it cannot have an import-time side effect. Rather than try to infer the type and find the corresponding builtin method, we just return early. (Without this fix we would get a false positive `unknown-method`). Reviewed By: brittanyrey Differential Revision: D108094683 fbshipit-source-id: b7cb0f7a9123bfb8ae6f403e6a87fa3e2e6c0a5e
1 parent 24abd90 commit 8722f77

2 files changed

Lines changed: 54 additions & 0 deletions

File tree

src/source_analyzer.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,25 @@ fn get_qualified_import(res: &ResolvedName, attribute_module: &ModuleName) -> Op
137137
Some(full_module)
138138
}
139139

140+
fn is_builtin_literal(expr: &Expr) -> bool {
141+
matches!(
142+
expr,
143+
Expr::StringLiteral(_)
144+
| Expr::BytesLiteral(_)
145+
| Expr::NumberLiteral(_)
146+
| Expr::BooleanLiteral(_)
147+
| Expr::FString(_)
148+
| Expr::TString(_)
149+
| Expr::List(_)
150+
| Expr::Dict(_)
151+
| Expr::Set(_)
152+
| Expr::Tuple(_)
153+
| Expr::ListComp(_)
154+
| Expr::SetComp(_)
155+
| Expr::DictComp(_)
156+
)
157+
}
158+
140159
/// Implementation of Analyzer for source files (.py).
141160
pub struct SourceAnalyzer<'a> {
142161
parsed_module: &'a ParsedModule,
@@ -319,6 +338,13 @@ impl<'a> SourceAnalyzer<'a> {
319338
}
320339

321340
fn unknown_function_name(&self, func: &Expr, output: &mut ModuleEffects) {
341+
// A method call whose receiver is a freshly-constructed builtin literal operates on a value
342+
// with no external aliasing, so it cannot have an import-time side effect.
343+
if let Expr::Attribute(e) = func
344+
&& is_builtin_literal(&e.value)
345+
{
346+
return;
347+
}
322348
trace!("Unknown call: {}(...)", format::format_expr(func));
323349
let name = match func {
324350
Expr::Attribute(e) => match *e.value {

tests/calls.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,34 @@ def f(x):
175175
check(code);
176176
}
177177

178+
#[test]
179+
fn test_method_call_on_literal_safe() {
180+
// A method call on a freshly-constructed builtin literal is safe.
181+
let code = r#"
182+
a = ":".join(["x", "y"])
183+
b = "hello".upper()
184+
c = [1, 2].index(1)
185+
d = {"k": 1}.get("k")
186+
e = (1, 2).count(1)
187+
f = {1, 2}.union({3})
188+
g = b"x".decode()
189+
h = f"v={a}".strip()
190+
i = [x for x in range(3)].pop()
191+
"#;
192+
check(code);
193+
}
194+
195+
#[test]
196+
fn test_method_call_on_literal_still_checks_args() {
197+
// The spurious unknown-method-call effect is suppressed, but an imported
198+
// argument passed to the literal's method is still flagged.
199+
let code = r#"
200+
import os
201+
[].append(os) # E: imported-var-argument
202+
"#;
203+
check_effects(code);
204+
}
205+
178206
#[test]
179207
fn test_imported_method_call() {
180208
let code1 = r#"

0 commit comments

Comments
 (0)