Skip to content

Commit

Permalink
[CI] Lint check for suspicious #includes (#14704)
Browse files Browse the repository at this point in the history
* [CI] Lint check for suspicious #includes

#### Problem

Code for embedded use should generally avoid using C++ library
features that heap allocate (e.g. STL containers) or have large data
(e.g. locale).

#### Change overview

`scripts/tools/check_includes.sh` does a simple search
for `#include` directives and compares them against rules
in `scripts/tools/check_includes_config.py`.

This can be run from a command line as well as the CI workflow.

#### Testing

Manual runs.

* Work with python3.8 in CI
  • Loading branch information
kpschoedel authored Feb 3, 2022
1 parent 0471a29 commit 11182e6
Show file tree
Hide file tree
Showing 4 changed files with 269 additions and 0 deletions.
6 changes: 6 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
102 changes: 102 additions & 0 deletions scripts/tools/check_includes.py
Original file line number Diff line number Diff line change
@@ -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<file>.+)
: (?P<line>\d+)
: (?P<directive>
\s* \# \s* include \s* (?P<type>[<"]) (?P<include>[^">]+) [">])
(?P<trailing> .*)
""", 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())
19 changes: 19 additions & 0 deletions scripts/tools/check_includes.sh
Original file line number Diff line number Diff line change
@@ -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"
142 changes: 142 additions & 0 deletions scripts/tools/check_includes_config.py
Original file line number Diff line number Diff line change
@@ -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 <chrono> 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'},

}

0 comments on commit 11182e6

Please sign in to comment.