Skip to content

fix support multiple values for python-platform #3440#3467

Open
asukaminato0721 wants to merge 6 commits into
facebook:mainfrom
asukaminato0721:3440
Open

fix support multiple values for python-platform #3440#3467
asukaminato0721 wants to merge 6 commits into
facebook:mainfrom
asukaminato0721:3440

Conversation

@asukaminato0721

@asukaminato0721 asukaminato0721 commented May 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #3440

PythonPlatform now can represent one platform, multiple platforms, or all.

Platform checks now only fold to true/false when all configured platforms agree; mixed results stay unknown so both branches are type checked.

Test Plan

add test

@meta-cla meta-cla Bot added the cla signed label May 19, 2026
@asukaminato0721 asukaminato0721 marked this pull request as ready for review May 19, 2026 17:39
Copilot AI review requested due to automatic review settings May 19, 2026 17:39

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds support for specifying multiple python-platform values (and an "all" option) so Pyrefly can avoid incorrectly pruning platform-gated branches when code is intended to run across multiple OS targets.

Changes:

  • Update PythonPlatform to represent all or a normalized set of platforms, with serde support for string-or-list config values.
  • Extend SysInfo expression evaluation so sys.platform / os.name comparisons can produce “unknown” results when multiple platforms are possible, plus add unit tests for these cases.
  • Update user docs and JSON schema to reflect the expanded python-platform configuration surface.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
website/docs/configuration.mdx Documents python-platform as string-or-list and introduces "all" semantics.
schemas/pyrefly.json Allows python-platform to be either a string or a non-empty array of strings.
crates/pyrefly_python/src/sys_info.rs Implements multi-platform/all semantics in PythonPlatform and updates condition evaluation accordingly, with new tests.
crates/pyrefly_config/src/config.rs Adds config parsing tests for list and "all" values.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/pyrefly_python/src/sys_info.rs Outdated
Comment thread crates/pyrefly_python/src/sys_info.rs Outdated
Comment thread schemas/pyrefly.json Outdated
Comment thread website/docs/configuration.mdx Outdated
@asukaminato0721 asukaminato0721 marked this pull request as draft May 19, 2026 18:07
@asukaminato0721 asukaminato0721 marked this pull request as ready for review May 19, 2026 18:11
@github-actions github-actions Bot added size/xl and removed size/xl labels May 19, 2026
Comment thread website/docs/configuration.mdx Outdated

@connernilsen connernilsen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey Asuka, thanks for making this. I have a few changes, but a few questions remain on some of the implementation details

Comment thread crates/pyrefly_python/src/sys_info.rs Outdated
Comment thread crates/pyrefly_config/src/config.rs
Comment thread crates/pyrefly_python/src/sys_info.rs Outdated
Comment thread crates/pyrefly_python/src/sys_info.rs Outdated
Comment thread crates/pyrefly_python/src/sys_info.rs
Comment thread crates/pyrefly_python/src/sys_info.rs
Comment thread crates/pyrefly_python/src/sys_info.rs Outdated
Comment thread crates/pyrefly_python/src/sys_info.rs Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

4 participants