Skip to content

Commit

Permalink
Check for empty sources for configured targets using `.get_attr("srcs…
Browse files Browse the repository at this point in the history
…")` instead of `.sources()`

Summary:
It's not exactly clear to me what's the difference between `configured_target.sources()` and `configured_target.get_attr("srcs")`. My intuition is that the former should just be a flattened list of the latter, but apparently it's not in practice as indicated by [user report](https://fb.workplace.com/groups/pyreqa/permalink/9054752214614530/) -- the target being reported is `fbcode//thrift/lib/python/test:special_cases-python-types`.

Asked on BXL group [here](https://fb.workplace.com/groups/617497306123691/permalink/1104965664043517/) but in the meantime I think we'll just go with the get_attr approach since [`srcs` is what was used to calculate library manifests](https://www.internalfb.com/code/fbsource/[316b867a4bea]/fbcode/buck2/prelude/python/python_library.bzl?lines=213&base=4dc2d9f67ecd3b31c6b5909d83be85180ec3d81a). This is still not 100% perfect, as we are not yet considering platform srcs. Some future work might be needed for that.

Reviewed By: IanChilds

Differential Revision: D67294119

fbshipit-source-id: 9e8c9e34d36ff963d9887a761fbd0133c35e8989
  • Loading branch information
grievejia authored and facebook-github-bot committed Dec 17, 2024
1 parent 3f5d622 commit 04c932f
Showing 1 changed file with 8 additions and 3 deletions.
11 changes: 8 additions & 3 deletions python/typecheck/batch.bxl
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,15 @@ def _should_type_check_unconfigured(unconfigured_target: bxl.UnconfiguredTargetN
return _is_python_target(unconfigured_target.rule_type)

def _should_type_check_configured(configured_target: bxl.ConfiguredTargetNode, analysis_result: bxl.AnalysisResult) -> bool:
# NOTE(grievejia): This is NOT the same as `configured_target.sources()`.
# See https://fb.workplace.com/groups/617497306123691/permalink/1104965664043517/
srcs = configured_target.get_attr("srcs")
return (
# NOTE(grievejia): This is NOT the same as checking `srcs` on the unconfigured target, as we won't be
# able to, e.g., rule out generated/non-existent sources on unconfigured targets.
len(configured_target.sources()) > 0 and
# NOTE(grievejia): This is NOT the same as checking `srcs` on the unconfigured target, as
# empty `srcs` on unconfigured target does not necessarily imply empty `srcs` on configured
# target (due to, e.g. `select()`).
srcs != None and
len(srcs) > 0 and
"typecheck" in analysis_result.providers()[DefaultInfo].sub_targets
)

Expand Down

0 comments on commit 04c932f

Please sign in to comment.