pw_presubmit: Add check for TODO() format
Change-Id: If77b3cbd35d2ad09d2892453aaafd17df9fc1cbd
Bug: b/242060498
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/105930
Commit-Queue: Auto-Submit <auto-submit@pigweed.google.com.iam.gserviceaccount.com>
Pigweed-Auto-Submit: Rob Mohr <mohrr@google.com>
Reviewed-by: Wyatt Hepler <hepler@google.com>
diff --git a/pw_presubmit/docs.rst b/pw_presubmit/docs.rst
index e9a393e..a5919cb 100644
--- a/pw_presubmit/docs.rst
+++ b/pw_presubmit/docs.rst
@@ -124,6 +124,19 @@
C/C++ headers is ``#pragma once``. This is enabled by adding
``pw_presubmit.pragma_once`` to a presubmit program.
+.. todo-check: disable
+
+TODO(b/###) Formatting
+^^^^^^^^^^^^^^^^^^^^^^^^^
+There's a check that confirms ``TODO`` lines match a given format. Upstream
+Pigweed expects these to look like ``TODO(b/###): Explanation``, but makes it
+easy for projects to define their own pattern instead.
+
+To use this check add ``todo_check.create(todo_check.BUGS_OR_USERNAMES)`` to a
+presubmit program.
+
+.. todo-check: enable
+
Python Checks
^^^^^^^^^^^^^
There are two checks in the ``pw_presubmit.python_checks`` module, ``gn_pylint``
diff --git a/pw_presubmit/py/BUILD.gn b/pw_presubmit/py/BUILD.gn
index ee0712c..def2262 100644
--- a/pw_presubmit/py/BUILD.gn
+++ b/pw_presubmit/py/BUILD.gn
@@ -36,6 +36,7 @@
"pw_presubmit/presubmit.py",
"pw_presubmit/python_checks.py",
"pw_presubmit/shell_checks.py",
+ "pw_presubmit/todo_check.py",
"pw_presubmit/tools.py",
]
tests = [
diff --git a/pw_presubmit/py/pw_presubmit/pigweed_presubmit.py b/pw_presubmit/py/pw_presubmit/pigweed_presubmit.py
index f33a44f..910d628 100755
--- a/pw_presubmit/py/pw_presubmit/pigweed_presubmit.py
+++ b/pw_presubmit/py/pw_presubmit/pigweed_presubmit.py
@@ -46,6 +46,7 @@
call,
filter_paths,
inclusive_language,
+ npm_presubmit,
plural,
presubmit,
PresubmitContext,
@@ -53,7 +54,7 @@
Programs,
python_checks,
shell_checks,
- npm_presubmit,
+ todo_check,
)
from pw_presubmit.install_hook import install_git_hook
@@ -350,13 +351,13 @@
'-//pw_blob_store/...:all',
'-//pw_boot/...:all',
'-//pw_cpu_exception_cortex_m/...:all',
- '-//pw_crypto/...:all', # TODO(b/236321905)
+ '-//pw_crypto/...:all', # TODO(b/236321905) Remove when passing.
'-//pw_file/...:all',
- '-//pw_function:function_test', # TODO(b/241821115)
- '-//pw_hdlc/rpc_example', # TODO(b/241575924)
+ '-//pw_function:function_test', # TODO(b/241821115) Remove when passing.
+ '-//pw_hdlc/rpc_example', # TODO(b/241575924) Remove when passing.
'-//pw_i2c_mcuxpresso/...:all',
'-//pw_kvs/...:all',
- '-//pw_log:log_proto_py_pb2', # TODO(b/241456982)
+ '-//pw_log:log_proto_py_pb2', # TODO(b/241456982) Remove when passing.
'-//pw_log:log_proto_py_pb2_genproto',
'-//pw_log_null/...:all',
'-//pw_log_string/...:all',
@@ -389,7 +390,8 @@
'-//pw_tls_client/...:all',
'-//pw_tls_client_boringssl/...:all',
'-//pw_tls_client_mbedtls/...:all',
- '-//pw_tokenizer:tokenizer_proto_py_pb2', # TODO(b/241456982)
+ # TODO(b/241456982) Remove when passing.
+ '-//pw_tokenizer:tokenizer_proto_py_pb2',
'-//pw_tokenizer:tokenizer_proto_py_pb2_genproto',
'-//pw_trace/...:all',
'-//pw_trace_tokenized/...:all',
@@ -750,6 +752,7 @@
static_analysis,
stm32f429i,
npm_presubmit.npm_test,
+ todo_check.create(todo_check.BUGS_OR_USERNAMES),
)
CRYPTO = (
diff --git a/pw_presubmit/py/pw_presubmit/todo_check.py b/pw_presubmit/py/pw_presubmit/todo_check.py
new file mode 100644
index 0000000..b7e9e30
--- /dev/null
+++ b/pw_presubmit/py/pw_presubmit/todo_check.py
@@ -0,0 +1,78 @@
+# Copyright 2022 The Pigweed Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not
+# use this file except in compliance with the License. You may obtain a copy of
+# the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations under
+# the License.
+"""Check the formatting of TODOs."""
+
+import re
+from typing import Sequence
+
+from pw_presubmit import PresubmitContext, filter_paths
+
+EXCLUDE: Sequence[str] = (
+ # Metadata
+ r'^docker/tag$',
+ r'\byarn.lock$',
+ # Data files
+ r'\.bin$',
+ r'\.csv$',
+ r'\.elf$',
+ r'\.gif$',
+ r'\.jpg$',
+ r'\.json$',
+ r'\.png$',
+ r'\.svg$',
+ r'\.xml$',
+)
+
+# todo-check: disable
+BUGS_ONLY = re.compile(r'\bTODO\(b/\d+\).*\w')
+BUGS_OR_USERNAMES = re.compile(r'\bTODO\((?:b/\d+|[a-z]+)\).*\w')
+_TODO = re.compile(r'\bTODO\b')
+# todo-check: enable
+
+# If seen, ignore this line and the next.
+_IGNORE = 'todo-check: ignore'
+
+# Ignore a whole section. Please do not change the order of these lines.
+_DISABLE = 'todo-check: disable'
+_ENABLE = 'todo-check: enable'
+
+
+def create(todo_pattern: re.Pattern = BUGS_ONLY,
+ exclude: Sequence[str] = EXCLUDE):
+ """Create a todo_check presubmit step that uses the given pattern."""
+ @filter_paths(exclude=exclude)
+ def todo_check(ctx: PresubmitContext):
+ """Presubmit check that makes sure todo lines match the pattern."""
+ for path in ctx.paths:
+ with open(path) as file:
+ enabled = True
+ prev = ''
+
+ for i, line in enumerate(file, 1):
+ if _DISABLE in line:
+ enabled = False
+ elif _ENABLE in line:
+ enabled = True
+
+ if enabled:
+ if _IGNORE not in line and _IGNORE not in prev:
+ if _TODO.search(line):
+ if not todo_pattern.search(line):
+ # todo-check: ignore
+ ctx.fail(f'Bad TODO on line {i}:', path)
+ ctx.fail(f' {line.strip()}')
+
+ prev = line
+
+ return todo_check
