From 04c932faaa8d2b2b4fa8cf0acd0a94dcc1ac2b24 Mon Sep 17 00:00:00 2001 From: Jia Chen Date: Tue, 17 Dec 2024 15:19:47 -0800 Subject: [PATCH] Check for empty sources for configured targets using `.get_attr("srcs")` 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 --- python/typecheck/batch.bxl | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/python/typecheck/batch.bxl b/python/typecheck/batch.bxl index 07318d959..8774f2800 100644 --- a/python/typecheck/batch.bxl +++ b/python/typecheck/batch.bxl @@ -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 )