Skip to content

Commit 052d997

Browse files
nitishagarmeta-codesync[bot]
authored andcommitted
Report one bad-context-manager error per broken with-item
Summary: Fixes #5079 ## Problem A context manager that fails **both** protocol halves produced two `bad-context-manager` errors with an identical header and range: ```python from contextlib import asynccontextmanager asynccontextmanager async def async_context(): yield with async_context(): # TWO identical errors appear here print("This will cause an error at runtime") ``` Both errors read "Cannot use `_AsyncGeneratorContextManager[None]` as a context manager"; they differ only in the detail line (`__enter__` vs `__exit__` missing). The error collector intentionally does not merge diagnostics whose details differ, so the pair always displayed. ## Change `context_value` checks the protocol in two halves (enter, exit), each attributing failures to the with-item's range under the shared `bad-context-manager` context. Each half's diagnostics are now collected in their own delayed collector, and only the **first failing half's** errors are forwarded: - enter broken → the enter error survives (it carries the actionable "Did you mean `__aenter__`?" hint, which is exactly the fix a user needs for the issue's case); - enter fine → exit errors forwarded byte-for-byte as before, including the existing two-call consolidation for bad `__exit__` signatures; - the exit half is still fully evaluated so its return type keeps feeding the `bool | None` check. An object failing only one half still yields exactly one error naming that half; `async with` on async context managers is unaffected. X-link: #5095 Reviewed By: rchen152 Differential Revision: D123266669 fbshipit-source-id: 036daaf3c0383dbab4f28f76fe7b5997a0782d0d
1 parent 8c07754 commit 052d997

3 files changed

Lines changed: 43 additions & 19 deletions

File tree

‎pyrefly/lib/alt/solve.rs‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2149,10 +2149,31 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
21492149
self.distribute_over_union(context_manager_type, |context_manager_type| {
21502150
let context =
21512151
|| ErrorContext::BadContextManager(self.for_display(context_manager_type.clone()));
2152-
let enter_type =
2153-
self.context_value_enter(context_manager_type, kind, range, errors, Some(&context));
2154-
let exit =
2155-
self.context_value_exit(context_manager_type, kind, range, errors, Some(&context));
2152+
// Collect each protocol half's errors separately: when both halves fail (e.g. an
2153+
// async context manager in a sync `with`), both would otherwise surface as
2154+
// identical-header `bad-context-manager` errors on the same range. The enter
2155+
// half's error wins, since it is the first protocol failure in reading order.
2156+
let enter_errors = ErrorCollector::new(errors.module().clone(), ErrorStyle::Delayed);
2157+
let enter_type = self.context_value_enter(
2158+
context_manager_type,
2159+
kind,
2160+
range,
2161+
&enter_errors,
2162+
Some(&context),
2163+
);
2164+
let exit_errors = ErrorCollector::new(errors.module().clone(), ErrorStyle::Delayed);
2165+
let exit = self.context_value_exit(
2166+
context_manager_type,
2167+
kind,
2168+
range,
2169+
&exit_errors,
2170+
Some(&context),
2171+
);
2172+
if !enter_errors.is_empty() {
2173+
errors.extend(enter_errors);
2174+
} else {
2175+
errors.extend(exit_errors);
2176+
}
21562177
let exit_type = self.union(exit.with_exception, exit.without_exception);
21572178
self.check_type(
21582179
&exit_type,

‎pyrefly/lib/test/lsp/lsp_interaction/diagnostic.rs‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -928,19 +928,6 @@ fn test_error_documentation_links() {
928928
"severity": 1,
929929
"source": "Pyrefly"
930930
},
931-
{
932-
"code": "bad-context-manager",
933-
"codeDescription": {
934-
"href": "https://pyrefly.org/en/docs/error-kinds/#bad-context-manager"
935-
},
936-
"message": "Cannot use `A` as a context manager\n Object of class `A` has no attribute `__exit__`",
937-
"range": {
938-
"end": {"character": 8, "line": 17},
939-
"start": {"character": 5, "line": 17}
940-
},
941-
"severity": 1,
942-
"source": "Pyrefly"
943-
},
944931
{
945932
"code": "missing-attribute",
946933
"codeDescription": {

‎pyrefly/lib/test/with.rs‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -571,15 +571,31 @@ testcase!(
571571
test_simple_with_error,
572572
r#"
573573
def test_sync() -> None:
574-
with 42 as foo: # E: Cannot use `Literal[42]` as a context manager\n Object of class `int` has no attribute `__enter__` # E: has no attribute `__exit__`
574+
with 42 as foo: # E: Cannot use `Literal[42]` as a context manager\n Object of class `int` has no attribute `__enter__`\n Did you mean `__new__`?
575575
pass
576576
577577
async def test_async() -> None:
578-
async with "abc" as bar: # E: has no attribute `__aenter__` # E: has no attribute `__aexit__`
578+
async with "abc" as bar: # E: Cannot use `Literal['abc']` as a context manager\n Object of class `str` has no attribute `__aenter__`\n Did you mean `__iter__`?
579579
pass
580580
"#,
581581
);
582582

583+
// An async context manager used in a sync `with` breaks both protocol halves at
584+
// the same range; only one `bad-context-manager` error should be reported.
585+
testcase!(
586+
test_async_context_manager_in_sync_with,
587+
r#"
588+
from contextlib import asynccontextmanager
589+
590+
@asynccontextmanager
591+
async def async_context():
592+
yield
593+
594+
with async_context(): # E: Cannot use `_AsyncGeneratorContextManager[None]` as a context manager\n Object of class `_AsyncGeneratorContextManager` has no attribute `__enter__`\n Did you mean `__aenter__`?
595+
pass
596+
"#,
597+
);
598+
583599
testcase!(
584600
test_simple_with_wrong_enter_type,
585601
r#"

0 commit comments

Comments
 (0)