Skip to content

Commit 3a261cc

Browse files
jorenhammeta-codesync[bot]
authored andcommitted
Special-case logging.Logger and re.Pattern in pyrefly coverage (#5552)
Summary: I noticed in several projects now that they tend to leave a module-level `logger = logging.getLogger()` unannotated in modules without `__all__`, and don't consider it to be public API. But `pyrefly coverage check`, due to `logger` not having a leading underscore, will consider it as untyped typable regardless. This is a common enough pattern that I think it's worth special-casing `logging.getLogger` to count it as implicitly typed when unannotated. To confirm my assumptions,I had an agent do a quick code survey (on my local `uv cache`), and it found 4,204 `logging.getLogger` calls of which only 108 were annotated and public. For `re.compile` it's very similar, with 137 annotated public module-level `re.compile` names out of 3,902 total `re.compile` calls, so I think that one is also worth special-casing. I had the agent look for other similar module-level constructor calls, but these two were by far the most common. - [x] I am a human, and I fully understand the code I am submitting. Pull Request resolved: #5552 Test Plan: Integration tests added Reviewed By: stroxler Differential Revision: D129909548 Pulled By: rchen152 fbshipit-source-id: 7aec2051bd093ed9e3b278b079a0b3cf58bd8ff6
1 parent 49d91ed commit 3a261cc

5 files changed

Lines changed: 166 additions & 5 deletions

File tree

‎crates/pyrefly_types/src/known_class.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ known_classes! {
171171
FastApi => ("fastapi", "FastAPI"),
172172
FastApiApplications => ("fastapi.applications", "FastAPI"),
173173
FastApiTestClient => ("fastapi.testclient", "TestClient"),
174+
LoggingLogger => ("logging", "Logger"),
174175
MarshmallowSchema => ("marshmallow.schema", "Schema"),
175176
NarwhalsDataFrame => ("narwhals.dataframe", "DataFrame"),
176177
NarwhalsLazyFrame => ("narwhals.dataframe", "LazyFrame"),

‎pyrefly/lib/commands/coverage/collect.rs‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,9 @@ const IMPLICIT_BUILTIN_CONSTRUCTORS: &[KnownClass] = &[
466466
KnownClass::Tuple,
467467
];
468468

469+
/// Stdlib classes whose instances are conventionally left unannotated, as `(module, class)`.
470+
const IMPLICIT_STDLIB_CLASSES: &[KnownClass] = &[KnownClass::LoggingLogger, KnownClass::RePattern];
471+
469472
fn parse_variables(
470473
module: &Module,
471474
answers: &Answers,
@@ -478,7 +481,7 @@ fn parse_variables(
478481

479482
/// Only a call hides its type at the assignment site, unless the callee names the class it
480483
/// yields: a builtin constructor like `list()`, or an explicitly specialized class like
481-
/// `Spam[int]()`.
484+
/// `Spam[int]()`. Instances of `IMPLICIT_STDLIB_CLASSES` are exempt whatever the call.
482485
fn untyped_if_call(answers: &Answers, idx: Idx<Key>, expr: &Expr) -> SlotCounts {
483486
let Expr::Call(call) = expr else {
484487
return SlotCounts::default();
@@ -498,10 +501,11 @@ fn parse_variables(
498501
_ => false,
499502
};
500503

501-
if answers
502-
.get_idx(idx)
503-
.is_some_and(|t| matches!(t.ty(), Type::ClassType(cls) if callee_names(cls)))
504-
{
504+
let is_implicit_stdlib =
505+
|cls: &ClassType| IMPLICIT_STDLIB_CLASSES.iter().any(|class| cls.is(*class));
506+
if answers.get_idx(idx).is_some_and(|t| {
507+
matches!(t.ty(), Type::ClassType(cls) if callee_names(cls) || is_implicit_stdlib(cls))
508+
}) {
505509
SlotCounts::default()
506510
} else {
507511
SlotCounts::untyped()
@@ -2326,6 +2330,14 @@ mod tests {
23262330
compare_snapshot("variables.expected.json", &report);
23272331
}
23282332

2333+
/// Calls returning a `logging.Logger` or `re.Pattern` have 0 slots, whatever the callee is.
2334+
/// Other calls count, even when the callee has the name of a stdlib factory.
2335+
#[test]
2336+
fn test_report_implicit_stdlib_classes() {
2337+
let report = build_module_report_for_test("implicit_stdlib_classes.py");
2338+
compare_snapshot("implicit_stdlib_classes.expected.json", &report);
2339+
}
2340+
23292341
/// gh-3773: optional imports must not be reported as untyped variables.
23302342
#[test]
23312343
fn test_report_optional_imports() {
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
{
2+
"name": "test",
3+
"path": "test.py",
4+
"names": [
5+
"test.logger",
6+
"test.wrapped_logger",
7+
"test.PATTERN",
8+
"test.lookalike",
9+
"test.make_logger",
10+
"test.Lookalike.getLogger",
11+
"test.Lookalike"
12+
],
13+
"line_count": 24,
14+
"symbol_reports": [
15+
{
16+
"kind": "attr",
17+
"name": "test.logger",
18+
"n_typable": 0,
19+
"n_typed": 0,
20+
"n_any": 0,
21+
"n_untyped": 0,
22+
"location": {
23+
"line": 19,
24+
"column": 1
25+
}
26+
},
27+
{
28+
"kind": "attr",
29+
"name": "test.wrapped_logger",
30+
"n_typable": 0,
31+
"n_typed": 0,
32+
"n_any": 0,
33+
"n_untyped": 0,
34+
"location": {
35+
"line": 20,
36+
"column": 1
37+
}
38+
},
39+
{
40+
"kind": "attr",
41+
"name": "test.PATTERN",
42+
"n_typable": 0,
43+
"n_typed": 0,
44+
"n_any": 0,
45+
"n_untyped": 0,
46+
"location": {
47+
"line": 21,
48+
"column": 1
49+
}
50+
},
51+
{
52+
"kind": "attr",
53+
"name": "test.lookalike",
54+
"n_typable": 1,
55+
"n_typed": 0,
56+
"n_any": 0,
57+
"n_untyped": 1,
58+
"location": {
59+
"line": 23,
60+
"column": 1
61+
}
62+
},
63+
{
64+
"kind": "function",
65+
"name": "test.make_logger",
66+
"n_typable": 1,
67+
"n_typed": 1,
68+
"n_any": 0,
69+
"n_untyped": 0,
70+
"location": {
71+
"line": 10,
72+
"column": 1
73+
}
74+
},
75+
{
76+
"kind": "function",
77+
"name": "test.Lookalike.getLogger",
78+
"n_typable": 1,
79+
"n_typed": 1,
80+
"n_any": 0,
81+
"n_untyped": 0,
82+
"location": {
83+
"line": 15,
84+
"column": 5
85+
}
86+
},
87+
{
88+
"kind": "class",
89+
"name": "test.Lookalike",
90+
"n_typable": 0,
91+
"n_typed": 0,
92+
"n_any": 0,
93+
"n_untyped": 0,
94+
"location": {
95+
"line": 14,
96+
"column": 1
97+
}
98+
}
99+
],
100+
"type_ignores": [],
101+
"n_typable": 3,
102+
"n_typed": 2,
103+
"n_any": 0,
104+
"n_untyped": 1,
105+
"coverage": 66.66666666666666,
106+
"strict_coverage": 66.66666666666666,
107+
"n_functions": 1,
108+
"n_methods": 1,
109+
"n_function_params": 0,
110+
"n_method_params": 0,
111+
"n_classes": 1,
112+
"n_attrs": 4,
113+
"n_properties": 0,
114+
"n_type_ignores": 0
115+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# Copyright (c) Meta Platforms, Inc. and affiliates.
2+
#
3+
# This source code is licensed under the MIT license found in the
4+
# LICENSE file in the root directory of this source tree.
5+
6+
import logging
7+
import re
8+
9+
10+
def make_logger() -> logging.Logger:
11+
return logging.getLogger(__name__)
12+
13+
14+
class Lookalike:
15+
def getLogger(self) -> int:
16+
return 0
17+
18+
19+
logger = logging.getLogger(__name__)
20+
wrapped_logger = make_logger()
21+
PATTERN = re.compile("a+")
22+
23+
lookalike = Lookalike().getLogger()

‎website/docs/report.mdx‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,16 @@ Only public symbols count: names without a leading underscore, plus anything in
3535
stub is also analyzed (`--prefer-stubs=false` disables this). With `--public-only`, only symbols
3636
reachable from public modules via re-export chains count: the library's public API.
3737

38+
A simple unannotated assignment to a module-level variable only counts as a typable if the value is
39+
a call, since a call hides the type at the assignment site. Three kinds of call are exempt:
40+
41+
- Some builtin constructors, like `list()`.
42+
- Explicitly specialized classes, like `Spam[int]()`.
43+
- Anything that returns a `logging.Logger` or `re.Pattern`, since those are conventionally left
44+
unannotated.
45+
46+
A variable assigned in both branches of an `if`/`else` counts whatever the values.
47+
3848
### Cython
3949

4050
In [Cython pure Python mode](https://cython.readthedocs.io/en/latest/src/tutorial/pure.html),

0 commit comments

Comments
 (0)