diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 76a5338e768a99..38e733d28a18b6 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -39,10 +39,16 @@ jobs: # to fail (exit nonzero) on match. And we wasnt to exclude this file, # to avoid our grep regexp matching itself. - name: Check for incorrect error use in VerifyOrExit + if: always() run: | git grep -n "VerifyOrExit(.*, [A-Za-z]*_ERROR" -- './*' ':(exclude).github/workflows/lint.yml' && exit 1 || exit 0 # Comments like '{{! ... }}' should be used in zap files - name: Do not allow TODO in generated files + if: always() run: | git grep -n 'TODO:' -- ./zzz_generated './*/zap-generated/*' && exit 1 || exit 0 + + - name: Check for disallowed include files + if: always() + run: scripts/tools/check_includes.sh diff --git a/scripts/tools/check_includes.py b/scripts/tools/check_includes.py new file mode 100644 index 00000000000000..02f63582fd76fd --- /dev/null +++ b/scripts/tools/check_includes.py @@ -0,0 +1,102 @@ +#!/usr/bin/env python3 +# +# Copyright (c) 2022 Project CHIP 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 +# +# http://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 #include files. + +Reads from standard input the output of a `grep -n` for `#include`s; +see `check_includes.sh`. + +Uses the conditions defined in `check_includes_config.py`. +""" + +import re +import sys + +from typing import Iterable, Pattern + +import check_includes_config as config + + +# The input comes from `grep -n` and has the form +# filename:line:include-directive +# +# This RE does not handle C-style comments before the file name. +# So don't do that. +_MATCH_SPLIT_RE = re.compile(r""" + (?P.+) + : (?P\d+) + : (?P + \s* \# \s* include \s* (?P[<"]) (?P[^">]+) [">]) + (?P .*) + """, re.VERBOSE) + + +# Allow a temporary override so that a PR will not be blocked +# on first updating `check_includes_config.py`. +OVERRIDE = 'T' + 'ODO: update check_includes_config.py' + + +def any_re(res: Iterable[str]) -> Pattern: + """Given a list of RE strings, return an RE to match any of them.""" + return re.compile('|'.join((f'({i})' for i in res))) + + +def main(): + + ignore_re = any_re(config.IGNORE) + + n = 0 + for line in sys.stdin: + s = line.strip() + m = _MATCH_SPLIT_RE.fullmatch(s) + if not m: + print(f"Unrecognized input '{s}'", file=sys.stderr) + return 2 + + if OVERRIDE in m.group('trailing'): + continue + + filename, include = m.group('file', 'include') + + if ignore_re.search(filename): + continue + + if include not in config.DENY: + continue + + if include in config.ALLOW.get(filename, []): + continue + + n += 1 + if n == 1: + print('Disallowed:\n') + + line_number, directive = m.group('line', 'directive') + print(f' {filename}:{line_number}: {directive}') + + if n > 0: + print('\nIf a disallowed #include is legitimate, add an ALLOW rule to') + print(f' {config.__file__}') + print('and/or temporarily suppress this error by adding exactly') + print(f' {OVERRIDE}') + print('in a comment at the end of the #include.') + return 1 + + return 0 + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/scripts/tools/check_includes.sh b/scripts/tools/check_includes.sh new file mode 100755 index 00000000000000..76f797cbfb248c --- /dev/null +++ b/scripts/tools/check_includes.sh @@ -0,0 +1,19 @@ +#!/bin/sh +# +# Copyright (c) 2022 Project CHIP 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 +# +# http://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. +# + +git grep -n -E '^[[:space:]]*#[[:space:]]*include[[:space:]]*[<"]' src | + python "${0%.*}.py" diff --git a/scripts/tools/check_includes_config.py b/scripts/tools/check_includes_config.py new file mode 100644 index 00000000000000..c3be4b0e5dd6b9 --- /dev/null +++ b/scripts/tools/check_includes_config.py @@ -0,0 +1,142 @@ +# +# Copyright (c) 2022 Project CHIP 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 +# +# http://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. +# + +"""Configuration for #include checking.""" + +from typing import Dict, Set + + +# IGNORE lists source files that are not checked at all. +# +# Each entry is a string giving a Python regular expression, +# un-anchored and case sensitive. + +IGNORE: Set[str] = { + + '/examples/', + '/java/', + '/Jni', + '/pybindings/', + '/python/', + '/Test', + '/tests/', + '/tools/', + + # Platforms can opt in or out. + '/darwin/', + '/platform/Ameba/', + '/platform/android/', + '/platform/CYW30739/', + '/platform/Darwin/', + '/platform/EFR32/', + '/platform/ESP32/', + '/platform/fake/', + '/platform/Linux/', + '/platform/nxp/', + '/platform/Tizen/', + r'POSIX\.h$', +} + + +# DENY lists disallowed include files. + +DENY: Set[str] = { + + # C++ headers often unsuitable for small platforms. + 'chrono', + 'clocale', + 'coroutine', + 'deque', + 'exception', + 'forward_list', + 'fstream', + 'iomanip', + 'ios', + 'iostream', + 'istream', + 'list', + 'locale', + 'locale.h', + 'map', + 'multimap', + 'multiset', + 'ostream', + 'queue', + 'set', + 'sstream', + 'stdexcept', + 'streambuf', + 'string', + 'string_view', + 'syncstream', + 'unordered_map', + 'unordered_set', + 'vector', + + # CHIP headers using STL containers. + 'lib/support/CHIPListUtils.h', # uses std::set + 'src/platform/DeviceSafeQueue.h', # uses std::deque +} + + +# ALLOW describes exceptions to DENY. +# +# The dictionary key is the name of the file performing the #include. +# The value is a set of names allowed to be included from that file +# despite being in DENY. + +ALLOW: Dict[str, Set[str]] = { + + # Not intended for embedded clients (#11705). + 'src/app/AttributeCache.h': {'list', 'map', 'set', 'vector'}, + 'src/app/BufferedReadCallback.h': {'vector'}, + + # Itself in DENY. + 'src/lib/support/CHIPListUtils.h': {'set'}, + 'src/platform/DeviceSafeQueue.h': {'queue'}, + + # libevent itself is unsuitable for small platforms. + 'src/system/SystemLayerImplLibevent.h': {'list', 'vector'}, + + # Only uses for zero-cost types. + 'src/system/SystemClock.h': {'chrono'}, + 'src/platform/mbed/MbedEventTimeout.h': {'chrono'}, + + 'src/app/app-platform/ContentApp.h': {'list', 'string'}, + 'src/app/clusters/application-basic-server/application-basic-delegate.h': {'list'}, + 'src/app/clusters/application-basic-server/application-basic-server.cpp': {'list'}, + 'src/app/clusters/application-launcher-server/application-launcher-delegate.h': {'list'}, + 'src/app/clusters/audio-output-server/audio-output-delegate.h': {'list'}, + 'src/app/clusters/channel-server/channel-delegate.h': {'list'}, + 'src/app/clusters/content-launch-server/content-launch-delegate.h': {'list'}, + 'src/app/clusters/content-launch-server/content-launch-server.cpp': {'list'}, + 'src/app/clusters/media-input-server/media-input-delegate.h': {'list'}, + 'src/app/clusters/media-playback-server/media-playback-delegate.h': {'list'}, + 'src/app/clusters/target-navigator-server/target-navigator-delegate.h': {'list'}, + + 'src/setup_payload/AdditionalDataPayload.h': {'string'}, + 'src/setup_payload/AdditionalDataPayloadParser.cpp': {'vector'}, + 'src/setup_payload/Base38Decode.h': {'string', 'vector'}, + 'src/setup_payload/ManualSetupPayloadGenerator.h': {'string'}, + 'src/setup_payload/ManualSetupPayloadParser.cpp': {'string', 'vector'}, + 'src/setup_payload/ManualSetupPayloadParser.h': {'string'}, + 'src/setup_payload/QRCodeSetupPayloadParser.cpp': {'vector'}, + 'src/setup_payload/QRCodeSetupPayloadParser.h': {'string'}, + 'src/setup_payload/SetupPayloadHelper.cpp': {'fstream'}, + 'src/setup_payload/SetupPayloadHelper.h': {'string'}, + 'src/setup_payload/SetupPayload.h': {'map', 'string', 'vector'}, + +}