[bug] The `check-builtin-literals` hook is not exhaustive

[lmmx]

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:

• Commit
• check_builtin_literals.rs
• Original: check_builtin_literals.py

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)

[asottile]

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]

Aha! I couldn't work out why, I'll leave it with you then 🙏

[asottile]

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]

Oh! I hadn't noticed that, I'll send in a PR to make sure you're properly credited! ❤️

[lmmx]

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]

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