Repository navigation
[bug] The check-builtin-literals hook is not exhaustive #1201
Description
I reimplemented the same program using the new ruff_python_parser and ruff_python_ast crates in Rust, and it identified various examples of builtin literals that are missed by the pre-commit hook version:
Apache Airflow uses this hook, the idea is that it catches unnecessary builtin type constructor calls and suggests replacing them with their literal equivalents (list() to [] etc.).
pre-commit says it is OK, and that all files don't contain them
$ pre-commit run -a --config .pre-commit-config.yaml --verbose
Require literal syntax when initializing builtins........................Passed
- hook id: check-builtin-literals
- duration: 0.67s but with my Rust program I find many undetected!
airflow-core/tests/unit/serialization/test_serde.py:97:45: replace list() with []
airflow-core/tests/unit/utils/test_db_cleanup.py:91:25: replace dict() with {}
airflow-core/tests/unit/utils/test_db_cleanup.py:115:25: replace dict() with {}
providers/google/tests/unit/google/cloud/sensors/test_dataproc_metastore.py:111:48: replace dict() with {}
providers/google/tests/unit/google/cloud/sensors/test_dataproc_metastore.py:111:56: replace list() with []
providers/google/tests/unit/google/cloud/sensors/test_dataproc_metastore.py:111:64: replace tuple() with ()
airflow-core/tests/unit/dag_processing/test_collection.py:371:69: replace dict() with {}
airflow-core/tests/unit/dag_processing/test_collection.py:470:65: replace dict() with {}
airflow-core/tests/unit/dag_processing/test_collection.py:583:65: replace dict() with {}
airflow-core/tests/unit/dag_processing/test_collection.py:613:65: replace dict() with {}
providers/apache/cassandra/tests/unit/apache/cassandra/sensors/test_record.py:38:29: replace dict() with {}
providers/apache/cassandra/tests/unit/apache/cassandra/sensors/test_record.py:53:29: replace dict() with {}
providers/apache/cassandra/tests/unit/apache/cassandra/sensors/test_record.py:70:29: replace dict() with {}
providers/google/tests/unit/google/cloud/transfers/test_sql_to_gcs.py:119:42: replace dict() with {}
providers/google/tests/unit/google/cloud/transfers/test_sql_to_gcs.py:174:42: replace dict() with {}
providers/google/tests/unit/google/cloud/transfers/test_sql_to_gcs.py:218:42: replace dict() with {}
providers/google/tests/unit/google/cloud/transfers/test_sql_to_gcs.py:262:42: replace dict() with {}
providers/google/tests/unit/google/cloud/transfers/test_sql_to_gcs.py:307:42: replace dict() with {}
providers/google/tests/unit/google/cloud/transfers/test_sql_to_gcs.py:369:42: replace dict() with {}
providers/cncf/kubernetes/tests/unit/cncf/kubernetes/test_pod_generator.py:748:25: replace dict() with {}
providers/apache/cassandra/tests/unit/apache/cassandra/sensors/test_table.py:37:29: replace dict() with {}
providers/apache/cassandra/tests/unit/apache/cassandra/sensors/test_table.py:53:29: replace dict() with {}
providers/apache/cassandra/tests/unit/apache/cassandra/sensors/test_table.py:67:29: replace dict() with {}
task-sdk/tests/task_sdk/bases/test_operator.py:881:12: replace dict() with {}
providers/amazon/src/airflow/providers/amazon/aws/executors/ecs/ecs_executor_config.py:52:17: replace dict() with {}
providers/amazon/src/airflow/providers/amazon/aws/executors/batch/batch_executor_config.py:48:74: replace dict() with {}
providers/openlineage/tests/unit/openlineage/test_conf.py:267:11: replace dict() with {}
providers/apache/hdfs/tests/unit/apache/hdfs/sensors/test_web_hdfs.py:39:29: replace dict() with {}
providers/apache/hdfs/tests/unit/apache/hdfs/sensors/test_web_hdfs.py:55:29: replace dict() with {}
providers/apache/hdfs/tests/unit/apache/hdfs/sensors/test_web_hdfs.py:75:29: replace dict() with {}
providers/apache/hdfs/tests/unit/apache/hdfs/sensors/test_web_hdfs.py:93:29: replace dict() with {}
dev/breeze/tests/test_selective_checks.py:1173:18: replace tuple() with ()
providers/amazon/tests/unit/amazon/aws/sensors/test_s3.py:558:36: replace dict() with {}
providers/amazon/tests/unit/amazon/aws/sensors/test_s3.py:560:36: replace dict() with {}
providers/amazon/tests/unit/amazon/aws/sensors/test_s3.py:562:32: replace dict() with {}I think these are all valid hits:
louis 🌟 ~/tmp/airflow $ rg 'dict\(\)' airflow-core/tests/unit/utils/test_db_cleanup.py
91: pytest.param(dict(), True, id="not supplied"),
115: pytest.param(dict(), False, id="not supplied"),
louis 🌟 ~/tmp/airflow $ rg 'dict\(\)' airflow-core/tests/unit/dag_processing/test_collection.py
371: update_dag_parsing_results_in_db("testing", None, [dag], dict(), None, set(), session)
470: update_dag_parsing_results_in_db("testing", None, [dag], dict(), parse_duration, set(), session)
583: update_dag_parsing_results_in_db("testing", None, [dag], dict(), None, set(), session)
613: update_dag_parsing_results_in_db("testing", None, [dag], dict(), None, set(), session)
Activity
asottile commentedon Oct 15, 2025
yeah it appears to be missing a generic_visit call though arguably I'll delete it from here since it's only useful to python
lmmx commentedon Oct 15, 2025
Aha! I couldn't work out why, I'll leave it with you then 🙏
asottile commentedon Oct 16, 2025
via #1202 -- note that the linked repo above is violating the (very easy to follow) MIT license by not including a copy of our license in a derivative work
lmmx commentedon Oct 16, 2025
Oh! I hadn't noticed that, I'll send in a PR to make sure you're properly credited! ❤️
lmmx commentedon Oct 16, 2025
Hey Anthony, I sent in a PR (j178/prek#904) to add the Copyright line from your MIT license to his, but j178 said he'd actually already included it after being made aware previously.
I checked and they match: pre-commit LICENSE vs. prek licenses/LICENSE.pre-commit.txt.
I've built on top of MIT licensed projects myself before, and added my own copyright lines to the license, and in monorepo contexts I've seen licenses stored in the source tree like j178 has done there. spaCy for example stores a licenses/3rd_party_licenses.txt file (for numpy, scikit-learn, pyvi, etc.)
I'm guessing there might just be a misunderstanding, and you hadn't spotted that license file. If you'd rather it be in a different form please let me know and I can send another PR in!
asottile commentedon Oct 16, 2025
ah I see by sheer luck the pre-commit and pre-commit-hooks licenses are byte identical -- so "technically" they have the license for both repositories