Skip to content

Commit 3e3bcb4

Browse files
authored
Rewrite lint.sh as check_lint.py (#3161)
* Rewrite lint.sh as check_lint.py This makes several beneficial changes: * Runs in parallel: lint all now takes 5.5 seconds, down from 25. * Supports linting specific files, passed on the command-line. * Defaults to linting files changed since master, pass --all to lint everything. * Infers header language based on related files. * Lays the groundwork for linting more than just C++. * Adds support for linting python. * Fix lint errors * Use check_lint.py in check.sh * Remove lint.sh * Python lint configuration * Enable python linting in travis
1 parent 64d8a40 commit 3e3bcb4

15 files changed

+923
-117
lines changed

.gitignore

+3
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,9 @@ Ninja
7979
/cmake-build-debug
8080
/cmake-build-release
8181

82+
# Python
83+
*.pyc
84+
8285
# Visual Studio
8386
/.vs
8487

.travis.yml

+1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ jobs:
1616
before_install:
1717
- brew install clang-format
1818
- brew install swiftformat
19+
- pip install flake8
1920
script:
2021
- ./scripts/check.sh --test-only
2122

Firestore/Example/Benchmarks/FSTLevelDBBenchmarkTests.mm

+8-8
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,8 @@
1616

1717
#import <Foundation/Foundation.h>
1818
#import <XCTest/XCTest.h>
19-
#include <cstdint>
2019

21-
#include "benchmark/benchmark.h"
20+
#include <cstdint>
2221

2322
#import "Firestore/Source/Local/FSTLevelDB.h"
2423
#import "Firestore/Source/Local/FSTLocalSerializer.h"
@@ -29,6 +28,7 @@
2928
#include "Firestore/core/src/firebase/firestore/model/document_key.h"
3029
#include "Firestore/core/src/firebase/firestore/model/types.h"
3130
#include "Firestore/core/src/firebase/firestore/util/string_format.h"
31+
#include "benchmark/benchmark.h"
3232

3333
NS_ASSUME_NONNULL_BEGIN
3434

@@ -104,19 +104,19 @@ void FillDB() {
104104
auto docKey = DocumentKey::FromPathString(StringFormat("docs/doc_%i", i));
105105
std::string docKeyString = LevelDbRemoteDocumentKey::Key(docKey);
106106
txn.Put(docKeyString, DocumentData());
107-
WriteIndex(txn, docKey);
107+
WriteIndex(&txn, docKey);
108108
}
109109
txn.Commit();
110110
// Force a write to disk to simulate startup situation
111111
db_.ptr->CompactRange(NULL, NULL);
112112
}
113113

114114
protected:
115-
void WriteIndex(LevelDbTransaction &txn, const DocumentKey &docKey) {
115+
void WriteIndex(LevelDbTransaction *txn, const DocumentKey &docKey) {
116116
// Arbitrary target ID
117117
TargetId targetID = 1;
118-
txn.Put(LevelDbDocumentTargetKey::Key(docKey, targetID), emptyBuffer_);
119-
txn.Put(LevelDbTargetDocumentKey::Key(targetID, docKey), emptyBuffer_);
118+
txn->Put(LevelDbDocumentTargetKey::Key(docKey, targetID), emptyBuffer_);
119+
txn->Put(LevelDbTargetDocumentKey::Key(targetID, docKey), emptyBuffer_);
120120
}
121121

122122
FSTLevelDB *db_;
@@ -128,7 +128,7 @@ void WriteIndex(LevelDbTransaction &txn, const DocumentKey &docKey) {
128128
// Write a couple large values (documents)
129129
// In each test, either overwrite index entries and documents, or just documents
130130

131-
BENCHMARK_DEFINE_F(LevelDBFixture, RemoteEvent)(benchmark::State &state) {
131+
BENCHMARK_DEFINE_F(LevelDBFixture, RemoteEvent)(benchmark::State &state) { // NOLINT
132132
bool writeIndexes = static_cast<bool>(state.range(0));
133133
int64_t documentSize = state.range(1);
134134
int64_t docsToUpdate = state.range(2);
@@ -137,7 +137,7 @@ void WriteIndex(LevelDbTransaction &txn, const DocumentKey &docKey) {
137137
LevelDbTransaction txn(db_.ptr, "benchmark");
138138
for (int i = 0; i < docsToUpdate; i++) {
139139
auto docKey = DocumentKey::FromPathString(StringFormat("docs/doc_%i", i));
140-
if (writeIndexes) WriteIndex(txn, docKey);
140+
if (writeIndexes) WriteIndex(&txn, docKey);
141141
std::string docKeyString = LevelDbRemoteDocumentKey::Key(docKey);
142142
txn.Put(docKeyString, documentUpdate);
143143
}

scripts/binary_to_array.py

+10-6
Original file line numberDiff line numberDiff line change
@@ -50,19 +50,22 @@
5050

5151
arg_parser = argparse.ArgumentParser()
5252

53-
arg_parser.add_argument("input", help="Input file containing binary data to embed")
53+
arg_parser.add_argument("input",
54+
help="Input file containing binary data to embed")
5455
arg_parser.add_argument("--output_source",
55-
help="Output source file, defining the array data.")
56+
help="Output source file, defining the array data.")
5657
arg_parser.add_argument("--output_header",
57-
help="Output header file, declaring the array data.")
58+
help="Output header file, declaring the array data.")
5859
arg_parser.add_argument("--array", help="Identifier for the array.")
5960
arg_parser.add_argument("--array_size", help="Identifier for the array size.")
6061
arg_parser.add_argument("--filename", help="Override file name in code.")
61-
arg_parser.add_argument("--filename_identifier", help="Where to put the filename.")
62+
arg_parser.add_argument("--filename_identifier",
63+
help="Where to put the filename.")
6264
arg_parser.add_argument("--header_guard",
63-
help="Header guard to #define in the output header.")
65+
help="Header guard to #define in the output header.")
6466
arg_parser.add_argument("--cpp_namespace",
65-
help="C++ namespace to use. If blank, will generate a C array.")
67+
help="C++ namespace to use. "
68+
"If blank, will generate a C array.")
6669

6770
# How many hex bytes to display in a line. Each "0x00, " takes 6 characters, so
6871
# a width of 12 lets us fit within 80 characters.
@@ -278,5 +281,6 @@ def main():
278281
src.write(source_text)
279282
logging.debug("Wrote source file %s", output_source)
280283

284+
281285
if __name__ == "__main__":
282286
main()

scripts/check.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -261,4 +261,4 @@ fi
261261
"${top_dir}/scripts/check_test_inclusion.py"
262262

263263
# Google C++ style
264-
"${top_dir}/scripts/lint.sh" "${START_SHA}"
264+
"${top_dir}/scripts/check_lint.py" "${START_SHA}"

scripts/check_lint.py

+252
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,252 @@
1+
#!/usr/bin/env python
2+
3+
# Copyright 2019 Google
4+
#
5+
# Licensed under the Apache License, Version 2.0 (the "License");
6+
# you may not use this file except in compliance with the License.
7+
# You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
17+
"""Lints source files for conformance with the style guide that applies.
18+
19+
Currently supports linting Objective-C, Objective-C++, C++, and Python source.
20+
"""
21+
22+
import argparse
23+
import logging
24+
import os
25+
import re
26+
import subprocess
27+
import sys
28+
import textwrap
29+
30+
from lib import checker
31+
from lib import command_trace
32+
from lib import git
33+
from lib import source
34+
35+
_logger = logging.getLogger('lint')
36+
37+
38+
_dry_run = False
39+
40+
41+
_CPPLINT_OBJC_FILTERS = [
42+
# Objective-C uses #import and does not use header guards
43+
'-build/header_guard',
44+
45+
# Inline definitions of Objective-C blocks confuse
46+
'-readability/braces',
47+
48+
# C-style casts are acceptable in Objective-C++
49+
'-readability/casting',
50+
51+
# Objective-C needs use type 'long' for interop between types like NSInteger
52+
# and printf-style functions.
53+
'-runtime/int',
54+
55+
# cpplint is generally confused by Objective-C mixing with C++.
56+
# * Objective-C method invocations in a for loop make it think its a
57+
# range-for
58+
# * Objective-C dictionary literals confuse brace spacing
59+
# * Empty category declarations ("@interface Foo ()") look like function
60+
# invocations
61+
'-whitespace',
62+
]
63+
64+
_CPPLINT_OBJC_OPTIONS = [
65+
# cpplint normally excludes Objective-C++
66+
'--extensions=h,m,mm',
67+
68+
# Objective-C style allows longer lines
69+
'--linelength=100',
70+
71+
'--filter=' + ','.join(_CPPLINT_OBJC_FILTERS),
72+
]
73+
74+
75+
def main():
76+
global _dry_run
77+
78+
parser = argparse.ArgumentParser(description='Lint source files.')
79+
parser.add_argument('--dry-run', '-n', action='store_true',
80+
help='Show what the linter would do without doing it')
81+
parser.add_argument('--all', action='store_true',
82+
help='run the linter over all known sources')
83+
parser.add_argument('rev_or_files', nargs='*',
84+
help='A single revision that specifies a point in time '
85+
'from which to look for changes. Defaults to '
86+
'origin/master. Alternatively, a list of specific '
87+
'files or git pathspecs to lint.')
88+
args = command_trace.parse_args(parser)
89+
90+
if args.dry_run:
91+
_dry_run = True
92+
command_trace.enable_tracing()
93+
94+
pool = checker.Pool()
95+
96+
sources = _unique(source.CC_DIRS + source.OBJC_DIRS + source.PYTHON_DIRS)
97+
patterns = git.make_patterns(sources)
98+
99+
files = git.find_changed_or_files(args.all, args.rev_or_files, patterns)
100+
check(pool, files)
101+
102+
pool.exit()
103+
104+
105+
def check(pool, files):
106+
group = source.categorize_files(files)
107+
108+
for kind, files in group.kinds.items():
109+
for chunk in checker.shard(files):
110+
if not chunk:
111+
continue
112+
113+
linter = _linters[kind]
114+
pool.submit(linter, chunk)
115+
116+
117+
def lint_cc(files):
118+
return _run_cpplint([], files)
119+
120+
121+
def lint_objc(files):
122+
return _run_cpplint(_CPPLINT_OBJC_OPTIONS, files)
123+
124+
125+
def _run_cpplint(options, files):
126+
scripts_dir = os.path.dirname(os.path.abspath(__file__))
127+
cpplint = os.path.join(scripts_dir, 'cpplint.py')
128+
129+
command = [sys.executable, cpplint, '--quiet']
130+
command.extend(options)
131+
command.extend(files)
132+
133+
return _read_output(command)
134+
135+
136+
_flake8_warned = False
137+
138+
139+
def lint_py(files):
140+
flake8 = which('flake8')
141+
if flake8 is None:
142+
global _flake8_warned
143+
if not _flake8_warned:
144+
_flake8_warned = True
145+
_logger.warn(textwrap.dedent(
146+
"""
147+
Could not find flake8 on the path; skipping python lint.
148+
Install with:
149+
150+
pip install --user flake8
151+
"""))
152+
return
153+
154+
command = [flake8]
155+
command.extend(files)
156+
157+
return _read_output(command)
158+
159+
160+
def _read_output(command):
161+
command_trace.log(command)
162+
163+
if _dry_run:
164+
return checker.Result(0, '')
165+
166+
proc = subprocess.Popen(
167+
command, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
168+
output = proc.communicate('')[0]
169+
sc = proc.wait()
170+
171+
return checker.Result(sc, output)
172+
173+
174+
_linters = {
175+
'cc': lint_cc,
176+
'objc': lint_objc,
177+
'py': lint_py,
178+
}
179+
180+
181+
def _unique(items):
182+
return list(set(items))
183+
184+
185+
def make_path():
186+
"""Makes a list of paths to search for binaries.
187+
188+
Returns:
189+
A list of directories that can be sources of binaries to run. This includes
190+
both the PATH environment variable and any bin directories associated with
191+
python install locations.
192+
"""
193+
# Start with the system-supplied PATH.
194+
path = os.environ['PATH'].split(os.pathsep)
195+
196+
# In addition, add any bin directories near the lib directories in the python
197+
# path. This makes it possible to find flake8 in ~/Library/Python/2.7/bin
198+
# after pip install --user flake8. Also handle installations on Windows which
199+
# go in %APPDATA%/Python/Scripts.
200+
lib_pattern = re.compile(r'(.*)/[^/]*/site-packages')
201+
for entry in sys.path:
202+
entry = entry.replace(os.sep, '/')
203+
m = lib_pattern.match(entry)
204+
if not m:
205+
continue
206+
207+
python_root = m.group(1).replace('/', os.sep)
208+
209+
for bin_basename in ('bin', 'Scripts'):
210+
bin_dir = os.path.join(python_root, bin_basename)
211+
if bin_dir not in path and os.path.exists(bin_dir):
212+
path.append(bin_dir)
213+
214+
return path
215+
216+
217+
_PATH = make_path()
218+
219+
220+
def which(executable):
221+
"""Finds the executable with the given name.
222+
223+
Returns:
224+
The fully qualified path to the executable or None if the executable isn't
225+
found.
226+
"""
227+
if executable.startswith('/'):
228+
return executable
229+
230+
for executable_with_ext in _executable_names(executable):
231+
for entry in _PATH:
232+
joined = os.path.join(entry, executable_with_ext)
233+
if os.path.isfile(joined) and os.access(joined, os.X_OK):
234+
return joined
235+
236+
return None
237+
238+
239+
def _executable_names(executable):
240+
"""Yields a sequence of all possible executable names."""
241+
242+
if os.name == 'nt':
243+
pathext = os.environ.get('PATHEXT', '').split(os.pathsep)
244+
for ext in pathext:
245+
yield executable + ext
246+
247+
else:
248+
yield executable
249+
250+
251+
if __name__ == '__main__':
252+
main()

scripts/check_test_inclusion.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ def CheckProject(project_file, test_files):
8080
"""
8181

8282
# An dict of basename to filename
83-
basenames = {os.path.basename(f) : f for f in test_files}
83+
basenames = {os.path.basename(f): f for f in test_files}
8484

8585
file_list_pattern = re.compile(r"/\* (\S+) in Sources \*/")
8686
with open(project_file, "r") as fd:

scripts/lib/__init__.py

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
# Copyright 2019 Google
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.

0 commit comments

Comments
 (0)