Conner Nilsen

Add module finder options into struct Summary: We're passing a ton of settings into module finding functionality. Each time someone adds or updates a field, it results in a several hundred line change JUST from updating tests to add the default fields. Let's improve this by using structs that have default parameters instead ___ Differential Revision: D122169308 fbshipit-source-id: fb2423b745ffff4dcc13f974c967e72af5314006

Pass ModulePath instead of path to avoid applying build-system config to typeshed Summary: The build system supplied default config applies to every file Pyrefly knows about, which is likely not the intended behavior for most people around typeshed. For everything else, that seems like a reasonable choice. To make sure we don't apply it to typeshed, let's pass a `ModulePath` instead of just a `&Path` to all of the path-based config option resolvers, and within there, not apply the build system default config if it's a bundled typeshed `ModulePath` variant Reviewed By: kinto0 Differential Revision: D121447509 fbshipit-source-id: 96a2b50ef35cef8c6f627dbb5c3f2a87d18e4d80

Report statically dead `if` and `while` suites Summary: A suite disabled by a statically known condition was pruned for type checking and shaded in the editor, but never actually reported: ``` if False: print("unreachable") # no diagnostic ``` The only signal was an LSP-only hint, which never reached CLI output, so `pyrefly check` stayed silent on code that can never run. Report one diagnostic per disabled suite: the body of a statically false `if` or `while`, and every branch that an earlier chosen branch makes dead. The LSP-only hint is deleted in favour of the real diagnostic, which now carries the LSP `UNNECESSARY` tag so editors still gray the region out. A suite is only reported when its condition never consults the runtime environment. `sys.version_info`, `sys.platform`, `os.name`, and `TYPE_CHECKING` guards are deliberately left alone: such a suite is dead only under the configuration being checked and live under another, so reporting it would fire on every codebase that supports more than one Python version or platform. `SysInfo::depends_on_sys_info` already answers exactly that question, so using it collapses the previous platform-only check and the separate `TYPE_CHECKING` case into a single predicate and removes a bespoke helper. This is a deliberate divergence from BasedPyright, which does report version-guarded suites. A suite we decline to report still needs to look dead in an editor, so the editor-only hint that predated this diff is kept for exactly those ranges and dropped for the ones the real diagnostic covers. Greying comes from an `UNNECESSARY` tag on a diagnostic, and nothing else supplies it. Original implementation by shobhitmehro. For https://github.com/facebook/pyrefly/issues/3689 Reviewed By: rchen152 Differential Revision: D114991009 fbshipit-source-id: dafff106946bd18e0d81d746c52fae7a5a2806e7

Feature/update baseline file on save Summary: A baseline goes stale as soon as people start fixing the errors in it. Today the only way to tidy it is to remember to run `pyrefly check --prune-baseline` across the project, so in practice entries for errors that no longer exist accumulate and keep suppressing whatever later moves into their place. Add a project-level `baseline-auto-update` setting, off by default. When it is on, saving a file in the IDE rewrites that file's baseline without the entries the file no longer produces, so the baseline stays current as the work happens. Pruning on save is only safe if it never removes an entry that is still earning its keep, and a language server sees plenty of states where that is easy to get wrong. Four conditions keep it honest: - Only files saved since the last recheck are examined, and only their rows can be dropped. A row belonging to any other file is retained, including one whose file is missing from disk, because a save says nothing about files it did not look at. - A saved file that still has diagnostics of its own is skipped entirely. Part-way through a fix an entry can look unused only because the code does not type check yet, and dropping it would report the error as new once the fix lands. Only diagnostics at or above the severity the baseline was written at count, so a warning does not hold up an error-only baseline, and one half-fixed file does not hold up the others saved alongside it. - A file edited again after being saved is skipped until the next save. The recheck validates whatever the buffer holds when it reaches it, so its diagnostics describe unsaved text. - The prune runs inside the recheck that claimed the save, and the baseline is replaced by renaming a fully written temporary file over it. A reader racing the write sees the old baseline or the new one, never a partial file. Files are grouped by baseline path, so a batch parses and rewrites each baseline once however many of its files were saved together. `--prune-baseline` keeps its existing behaviour: a CLI check covers the whole project, so it can still treat a row whose file is gone as stale, which the new `StaleRowScope` makes explicit at the call site. Reviewed By: stroxler Differential Revision: D109265008 fbshipit-source-id: d47c46b67964bf0eee2ed9afabeb70e09b9ba297

Create benchmark suite for LSP memory usage Summary: Our existing benchmarks measure CLI type-checking speed and memory, and LSP go-to-definition latency, but nothing measures how much memory a language server holds during normal IDE use. This adds `scripts/benchmark/lsp_memory_benchmark.py`, which drives Pyrefly and basedpyright through a scripted IDE session (open files, find references, two rounds of edits) on four open-source projects. At each checkpoint it waits until memory stops changing and records it: physical footprint on macOS, PSS on Linux, summed over the server's process tree. basedpyright runs with its default `openFilesOnly` mode and with `workspace` mode, which is closer to Pyrefly's indexing scope. `lsp_memory_projects.json` pins the project commits, server versions, Python version, and dependency resolution date, so results can be reproduced externally. `summarize` prints per-checkpoint medians and ranges, counts basedpyright's heap-pressure cache clears, and with `--json` writes a small summary file that is safe to commit. ___ Differential Revision: D123502872 fbshipit-source-id: a97c46f11c9e5ff99896e442ea6cda80006cd937

Move basedpyright flag into pyright struct Summary: This moves the `is_basedpyright` flag into the `Pyright` struct to avoid passing an extra flag around to a lot of places that don't need access to it. It's mostly a refactor, with a little extra logic to set that flag when a basedpyright config is parsed. Reviewed By: kinto0 Differential Revision: D119595118 fbshipit-source-id: 525b504c0dea13fee79f791bf0b07e0cacbcbcce

Add `explicit-any-use` for values whose type is an explicit `Any` Summary: basedpyright's `reportAny` reports values whose type is an explicit `Any`: variables, arguments, attribute accesses, attributes and returned values. Pyrefly had nothing equivalent. `explicit-any` only reports where `Any` is written in an annotation, and the unknown-type checks deliberately ignore explicit `Any`, so the uses of an `Any`-typed value, including ones from library functions like `json.loads`, were never reported. This adds an `explicit-any-use` error kind, off by default and enabled only by the `all` preset. Each of the places that report an implicit `Any` - variables, arguments, attribute accesses, class attributes, values returned from functions without a return annotation, and lambdas - now reports an explicit `Any` under the new kind, so a value's `Any` is normally reported by just one of the three kinds. Which kind depends on the `Any` the value ends up with: a comparison, unary operator or augmented assignment on an explicit `Any` still produces an implicit `Any` and is reported as unknown; the next diff keeps the style there. Like `reportAny`, every such place is reported, including each attribute in a chain, and a loop, unpacking, `with` or comprehension target is reported at its name. Two positions that `reportAny` also covers are left to existing kinds rather than reported again here: a parameter annotated `Any` is reported as `explicit-any` on its annotation, and returning an explicit `Any` from a function with a concrete return annotation as `no-any-return-explicit`. A return annotation of `Any` is not reported, because it already tells the reader the value is unchecked, and neither is `object`, which forces the caller to narrow before use; a union with an `Any` member such as `int | Any` is also not reported, which is a gap recorded as a `bug` test. It is one kind for now; it can become an umbrella with a sub-kind per position later if projects want to switch off the noisier positions. The pyright migration maps `reportAny` to it. Two kinds of value that pyrefly itself invents were an explicit `Any`, so the new kind reported them as if the user had written `Any`. `attrs.fields()` on a class that isn't an attrs class reported an error and then returned an explicit `Any`; it now returns an error `Any`. The module globals `__builtins__`, `__loader__` and `__spec__` have types pyrefly does not model, with no annotation the user could change to make them known, so they are now an implicit `Any`; this also means they display as `Unknown` rather than `Any` in hovers and `reveal_type`. The `Any` in `__annotations__` and `__dict__` stays explicit, because typeshed declares those globals as `dict[str, Any]`. The three kinds are not perfectly disjoint. Over a 253,000-error corpus, 14 places report both `explicit-any-use` and one of the unknown kinds, so the same expression is described as both an explicit and an implicit `Any`. Twelve are lambda bodies, where the body appears to be checked more than once and its parameter takes a different style each time, and two are the `in` operator. All are confined to the `all` preset. The double evaluation looks to predate this diff, which is simply the first kind that makes it visible. Fixes https://github.com/facebook/pyrefly/issues/5122 ___ landed-with-radar-review Differential Revision: D125563412 fbshipit-source-id: bb75b7506659aa7b668a3065be02b859bb2ef074

Add a build-system-supplied default config Summary: Stack context: A build system that owns a repository's Python layout should be able to own its type checking settings too, so that users of a custom build system do not have to keep a parallel `pyrefly.toml` in sync with it. This stack lets the build system response carry named type checking configs, point each target at one, and name a default for everything else. - Adds a top-level `default_config` naming an entry in `configs`, supplying any setting a target's own config leaves unset, and covering files that belong to no target at all. - Without it, a build system can only describe targets it knows about. A file you open that is not in any target, and any setting a target config omits, still fall back to a config file — which is exactly the parallel configuration the stack is trying to remove. Resolution order is now sub-config -> target config -> `default_config` -> root config -> preset defaults. `default_config` names an entry in `configs` rather than inlining settings, so the default costs one entry no matter how many targets share it, and it is deserialized once like any other named config. A config name that `configs` does not define is now reported when the response is ingested. Such a name resolves to no settings, which is indistinguishable from a config that sets nothing, so a typo in a target's `config` or in `default_config` would otherwise silently fall back to the config file with no diagnostic. Reviewed By: kinto0 Differential Revision: D118368691 fbshipit-source-id: ad4731b163c9bb9b46533be9311fa81b2d471c27

Align each file's diagnostics against its baseline rows Summary: `column-ordered` currently behaves exactly like `column`, because baseline matching treats the baseline as a set of keys: the first matching entry suppresses every diagnostic sharing that key, however many there are. Compare each file's diagnostics against its baseline rows as an ordered sequence instead, and report the diagnostics the alignment could not pair off. Because the rows and the diagnostics are both in source order and neither key carries a line number, an unrelated diagnostic appearing earlier in the file shifts nothing: the surrounding keys still line up and only the new one is left over. That last property is the reason for aligning rather than comparing per-key totals. Totals would tell you *how many* diagnostics are new but not *which*, so inserting a diagnostic in the middle of a file would report one at the last line sharing its key — a line the author never touched. Aligning reports it where it was added. Alignment is also what makes a row's absence meaningful, which the next diffs build on. The alignment itself is `similar`'s Myers implementation, which is already vendored and already handles the prefix and suffix trimming this would otherwise hand-roll. It deliberately uses the plain entry point rather than the deadline-bounded one: whether a diagnostic is reported decides whether a check passes, so the same inputs have to give the same answer on every machine. Performance, measured in optimized builds against trunk: for baselines of 5k to 20k entries, unchanged or spread across files, `check`, `--update-baseline` and `--prune-baseline` are within noise of trunk and of running with no baseline; matching is a few milliseconds of a run dominated by type checking. The cost shows up only when one file with a large baseline is heavily rearranged: shuffling a 20k-diagnostic file added about 150 ms end to end (686 ms against 536 ms), and matching alone takes about 0.3 s for 20k rows and 2.7 s for 100k in a single file. It grows roughly as N^1.5 rather than N², because `similar`'s default search bounds its work by input size rather than time, which also keeps the result deterministic. Two consequences worth knowing. The order of rows within a file is now part of the comparison, so a `column-ordered` baseline must not be re-sorted by hand; `--update-baseline` already writes rows in source order. And a file whose diagnostics have been reordered relative to the baseline can report one as new, which a set comparison would not — the cost of knowing which diagnostic is which. At scale this is significant: after shuffling a 20k-diagnostic file, about half its diagnostics were reported as new, so a large refactor of a heavily baselined file means regenerating the baseline. Both baseline processors match through the shared `BaselineIndex`, so this is one new way of looking rows up there rather than a change to each processor. Reviewed By: rchen152 Differential Revision: D121235766 fbshipit-source-id: cecf4aa14fa6d225832734f7937087d21158e440

Require an owner binding for ordinary lambda parameters Summary: https://github.com/facebook/pyrefly/issues/5169 came from a lambda whose parameter binding had no owner. A lambda parameter's contextual type is installed in thread-local state while its owner (the binding of the enclosing expression) is solved, and reading the parameter forces the owner first. Without an owner, nothing guarantees that the type is installed before it is read, so bindings in the lambda body, such as a comprehension, could see an implicit `Any` depending on solve order. The previous diff fixed this for `Annotated` metadata, but owner-less lambdas remained elsewhere: - Conditions checked by `BindingExpect::Bool` or `BindingExpect::BranchSuiteReachability` (`if`, `elif`, `while`, `assert` and match guards) were bound without a current binding. For example, `assert check(lambda v: [e for e in v])` with `check(f: Callable[[int], object])` never reported that `int` is not iterable, while the same call in an assignment did. - Inside metadata, ternary tests, boolean operands, and comprehension conditions are bound with a non-pinning value usage derived from the static one, so their lambdas still got owner-less `LambdaParameter` bindings. Conditions are now bound under a `Key::UsageLink` that links to the `KeyExpect` that checks them, so solving the link infers the condition and the link owns its lambdas. An `if` statement links to its `BranchSuiteReachability` binding, which is now always inserted, even when every branch is statically skipped. With that, every value context that can contain a lambda provides a current binding, except decorators of a definition dropped by parse-error recovery, so the owner of `Binding::LambdaParameter` is now required. Lambdas without a current binding (static type information, expressions derived from it, and those decorators) get `StaticLambdaParameter` bindings, which are always an implicit `Any`. ___ landed-with-radar-review Differential Revision: D124866558 fbshipit-source-id: 73975259d372abec9f3d5e85b3bdfc1c0eb9ae0a