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
Copy link
Copy Markdown
Contributor

@asukaminato0721 asukaminato0721 commented May 19, 2026

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
Copy link
Copy Markdown

Copilot AI left a comment

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
Copy link
Copy Markdown
Contributor

@connernilsen connernilsen left a comment

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

support multiple values for python-platform

4 participants