Skip to content

Conversation

AryanBagade
Copy link
Contributor

@AryanBagade AryanBagade commented Aug 31, 2025

Closes #1015

Summary

  • Added ≥2-constraints validation for user-declared TypeVar(...) in pyrefly/lib/alt/expr.rs, matching the existing scoped-parameter rule in pyrefly/lib/alt/solve.rs.
  • Error text: “Expected at least 2 constraints in TypeVar T, got 1”.
  • De-duplicates diagnostics when both constraints and bound are present.

Tests

  • Added test_typevar_single_constraint_is_error in pyrefly/lib/test/generic_restrictions.rs.
  • All tests green: cargo +nightly-2025-06-20 test --all, python3 test.py.

Conformance

  • Ran python3 test.py; committed updates:
    • conformance/third_party/conformance.exp
    • conformance/third_party/conformance.result
    • conformance/third_party/results.json

Tooling

  • Formatted with the pinned nightly toolchain.
  • Clippy scoped to -p pyrefly is clean; workspace clippy warnings in pyrefly_config are pre-existing and out of scope.

Checklist

@meta-cla meta-cla bot added the cla signed label Aug 31, 2025
Copy link
Contributor

@yangdanny97 yangdanny97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR and the tests!

I understand the goal of wanting to avoid emitting two diagnostics during cases when a user has 1 constraint & a bound type, but I believe this case is going to be extremely rare, and not worth the increased complexity in the new logic.

Let me know if you disagree

Copy link
Contributor

@yangdanny97 yangdanny97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks cleaner. We'll try to get this merged on Tuesday.

@facebook-github-bot
Copy link
Contributor

@yangdanny97 has imported this pull request. If you are a Meta employee, you can view this in D81493328.

@AryanBagade
Copy link
Contributor Author

Hi @yangdanny97 , I think my branch has fallen behind main, which caused the conflict in conformance/third_party/results.json and led to the internal linter showing as pending/failing. Would you like me to rebase, resolve the conformance files, and re-run tests, or will you handle the conflict on your end?

Copy link
Contributor

@rchen152 rchen152 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review automatically exported from Phabricator review in Meta.

@AryanBagade
Copy link
Contributor Author

Review automatically exported from Phabricator review in Meta.

Got it, thanks for the approval. Since the conflict is only in conformance outputs, I’ll leave it to be regenerated on merge.

@rchen152
Copy link
Contributor

rchen152 commented Sep 2, 2025

Ah, sorry for missing your comment earlier! Yeah, we can rebase and regenerate the conformance output on our end.

@facebook-github-bot
Copy link
Contributor

@yangdanny97 merged this pull request in 99bab4a.

@yangdanny97
Copy link
Contributor

yeah no need to worry if the merge conflict is just in the generated files - those get automatically re-generated during the merge process

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disallow specifying only 1 constraint in a Type Var
4 participants