Add pyrefly type checker & lsp support (and fix defects)#23
Conversation
| error_response.append_header("content-type", b"text/plain") | ||
| error_message = f"Internal Server Error: {e}" | ||
| http_body.write(error_body, error_message.encode(), http_body.WriteEnd.BACK) | ||
| http_body.write(error_body, error_message.encode()) |
There was a problem hiding this comment.
This is a pretty good example of an error caught by the type checker.
Pyrefly is fast and the defaults seem to be a bit less aggressive than basedpyright. Ty is another option but it doesn't seem to be as mature, potentially. This adds pyrefly and introduces changes in order to get a clean run via `make check`. It does seem to have found a few bugs in my early testing; I stumbled upon it as it was an LSP option for zed. One thing it did seem to catch was the session reuse bug already fixed in upstream where we weren't checking await_request for None.
Previously, we fully removed and regenerated stubs for a bunch of stuff. Now, only do so based on changes to compute.wit which should generally be a good proxy. This also makes the lint checks much lighter by not requiring the we build all the examples just to get stubs.
This is a case where pyrefly was inferring the type incorrectly. Interestingly, I found that this was also the case with mypy and pyright as well, so just adding the cast. Other frameworks seem to have better support for typing than bottle.
posborne
force-pushed
the
posborne/pyrefly-type-checker
branch
from
005b89d to
929f1bc
Compare
erikrose
left a comment
erikrose
left a comment
There was a problem hiding this comment.
That extra list(…) is the only thing I really care about.
| # Default target builds all examples | ||
| all: $(COMPOSED_WASMS) | ||
|
|
||
| $(STUBS_DIR): $(COMPUTE_WIT) |
There was a problem hiding this comment.
Might I venture to suggest we instead use stubs/wit_world/__init__.py as the target? Using a dir as a target tends toward fragility, as adding or removing anything from the dir updates its modification date. That includes OS detritus like .DS_Store and things I lack the foresight to enumerate.
There was a problem hiding this comment.
Good call, I'll do that in some way. I recall similar problems now that you mention it (and never really having a solution I'm entirely happy with).
There was a problem hiding this comment.
I used to use Make targets to create virtualenvs. One strategy I'd use is to make the venv, then immediately touch some pointless little file inside it as a sentinel. The target would then be that sentinel. Avoids the dir modtime problem and doesn't assume anything about the eventual population of the folder.
There was a problem hiding this comment.
Looks like another option, which we employ for BUILD_DIR is a https://www.gnu.org/software/make/manual/make.html#Prerequisite-Types
I will go ahead and use that as it seems to be designed and documented for this sort of case specifically.
There was a problem hiding this comment.
Though BUILD_DIR's only care is to create an empty dir. Stubs, OTOH, can go out of date, and we want them rebuilt if compute.wit changes. (I guess what I'm saying is that I wonder if | is transitive.)
There was a problem hiding this comment.
I pushed a change using | now, though I just saw the updates here concurrently.
Doing a test, I think the behavior is what we want:
$ make clean
$ make stubs
rm -rf stubs
uv run componentize-py -d wit -w fastly:compute/service bindings stubs
$ touch wit/deps/fastly/compute.wit
$ make stubs
rm -rf stubs
uv run componentize-py -d wit -w fastly:compute/service bindings stubs
# depends on stubs as `|` but we didn't regen after modifying a file
# within stubs
$ touch stubs/foo.txt
$ make lint
uv run --extra dev ruff check .
All checks passed!
uv run --extra dev --extra test pyrefly check .
INFO 0 errors (1 suppressed)
# But we do regen if stubs was actually modified
$ touch wit/deps/fastly/compute.wit
$ make lint
rm -rf stubs
uv run componentize-py -d wit -w fastly:compute/service bindings stubs
uv run --extra dev ruff check .
All checks passed!
uv run --extra dev --extra test pyrefly check .
INFO 0 errors (1 suppressed)
There was a problem hiding this comment.
Going to merge; if we see cases where it isn't working as we want, we can revisit.
This is a feawture of GNU make designed and documented to help with directories and similar. See https://www.gnu.org/software/make/manual/html_node/Prerequisite-Types.html
2 participants
Pyrefly is fast and the defaults seem to be a bit less aggressive than basedpyright. Ty is another option but it doesn't seem to be as mature, potentially. This adds pyrefly and introduces changes in order to get a clean run via
make check.It does seem to have found a few bugs in my early testing; I stumbled upon it as it was an LSP option for zed.