Nico Weber | 077f1a3 | 2015-08-06 15:08:57 -0700 | [diff] [blame] | 1 | # Copyright 2015 The Chromium Authors. All rights reserved. |
| 2 | # Use of this source code is governed by a BSD-style license that can be |
| 3 | # found in the LICENSE file. |
| 4 | |
| 5 | """Presubmit script for pdfium. |
| 6 | |
| 7 | See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts |
| 8 | for more details about the presubmit API built into depot_tools. |
| 9 | """ |
| 10 | |
Dan Sinclair | 22d6607 | 2016-02-22 11:56:05 -0500 | [diff] [blame] | 11 | LINT_FILTERS = [ |
Dan Sinclair | 3ebd121 | 2016-03-09 09:59:23 -0500 | [diff] [blame] | 12 | # Rvalue ref checks are unreliable. |
dan sinclair | d2019df | 2016-02-22 22:32:03 -0500 | [diff] [blame] | 13 | '-build/c++11', |
Dan Sinclair | 3ebd121 | 2016-03-09 09:59:23 -0500 | [diff] [blame] | 14 | # Need to fix header names not matching cpp names. |
dan sinclair | d2019df | 2016-02-22 22:32:03 -0500 | [diff] [blame] | 15 | '-build/include_order', |
Dan Sinclair | 3ebd121 | 2016-03-09 09:59:23 -0500 | [diff] [blame] | 16 | # Too many to fix at the moment. |
dan sinclair | d2019df | 2016-02-22 22:32:03 -0500 | [diff] [blame] | 17 | '-readability/casting', |
Dan Sinclair | 3ebd121 | 2016-03-09 09:59:23 -0500 | [diff] [blame] | 18 | # Need to refactor large methods to fix. |
dan sinclair | d2019df | 2016-02-22 22:32:03 -0500 | [diff] [blame] | 19 | '-readability/fn_size', |
Dan Sinclair | 3ebd121 | 2016-03-09 09:59:23 -0500 | [diff] [blame] | 20 | # Lots of usage to fix first. |
dan sinclair | d2019df | 2016-02-22 22:32:03 -0500 | [diff] [blame] | 21 | '-runtime/int', |
Dan Sinclair | 3ebd121 | 2016-03-09 09:59:23 -0500 | [diff] [blame] | 22 | # Lots of non-const references need to be fixed |
dan sinclair | d2019df | 2016-02-22 22:32:03 -0500 | [diff] [blame] | 23 | '-runtime/references', |
Dan Sinclair | 3ebd121 | 2016-03-09 09:59:23 -0500 | [diff] [blame] | 24 | # We are not thread safe, so this will never pass. |
dan sinclair | d2019df | 2016-02-22 22:32:03 -0500 | [diff] [blame] | 25 | '-runtime/threadsafe_fn', |
Dan Sinclair | 3ebd121 | 2016-03-09 09:59:23 -0500 | [diff] [blame] | 26 | # Figure out how to deal with #defines that git cl format creates. |
dan sinclair | d2019df | 2016-02-22 22:32:03 -0500 | [diff] [blame] | 27 | '-whitespace/indent', |
Dan Sinclair | 22d6607 | 2016-02-22 11:56:05 -0500 | [diff] [blame] | 28 | ] |
| 29 | |
Dan Sinclair | 544bbc6 | 2016-03-14 15:07:39 -0400 | [diff] [blame] | 30 | |
dsinclair | 2ca2da5 | 2016-09-13 18:10:34 -0700 | [diff] [blame] | 31 | _INCLUDE_ORDER_WARNING = ( |
| 32 | 'Your #include order seems to be broken. Remember to use the right ' |
| 33 | 'collation (LC_COLLATE=C) and check\nhttps://google.github.io/styleguide/' |
| 34 | 'cppguide.html#Names_and_Order_of_Includes') |
| 35 | |
| 36 | |
Dan Sinclair | 544bbc6 | 2016-03-14 15:07:39 -0400 | [diff] [blame] | 37 | def _CheckUnwantedDependencies(input_api, output_api): |
| 38 | """Runs checkdeps on #include statements added in this |
| 39 | change. Breaking - rules is an error, breaking ! rules is a |
| 40 | warning. |
| 41 | """ |
| 42 | import sys |
| 43 | # We need to wait until we have an input_api object and use this |
| 44 | # roundabout construct to import checkdeps because this file is |
| 45 | # eval-ed and thus doesn't have __file__. |
| 46 | original_sys_path = sys.path |
| 47 | try: |
Lei Zhang | b1d7872 | 2018-02-02 22:17:58 +0000 | [diff] [blame] | 48 | def GenerateCheckdepsPath(base_path): |
| 49 | return input_api.os_path.join(base_path, 'buildtools', 'checkdeps') |
| 50 | |
| 51 | presubmit_path = input_api.PresubmitLocalPath() |
| 52 | presubmit_parent_path = input_api.os_path.dirname(presubmit_path) |
| 53 | not_standalone_pdfium = \ |
| 54 | input_api.os_path.basename(presubmit_parent_path) == "third_party" and \ |
| 55 | input_api.os_path.basename(presubmit_path) == "pdfium" |
| 56 | |
| 57 | sys.path.append(GenerateCheckdepsPath(presubmit_path)) |
| 58 | if not_standalone_pdfium: |
| 59 | presubmit_grandparent_path = input_api.os_path.dirname( |
| 60 | presubmit_parent_path) |
| 61 | sys.path.append(GenerateCheckdepsPath(presubmit_grandparent_path)) |
| 62 | |
Dan Sinclair | 544bbc6 | 2016-03-14 15:07:39 -0400 | [diff] [blame] | 63 | import checkdeps |
| 64 | from cpp_checker import CppChecker |
| 65 | from rules import Rule |
dsinclair | 4cd49e1 | 2016-04-05 10:28:48 -0700 | [diff] [blame] | 66 | except ImportError: |
| 67 | return [output_api.PresubmitError( |
| 68 | 'Unable to run checkdeps, does pdfium/buildtools/checkdeps exist?')] |
Dan Sinclair | 544bbc6 | 2016-03-14 15:07:39 -0400 | [diff] [blame] | 69 | finally: |
| 70 | # Restore sys.path to what it was before. |
| 71 | sys.path = original_sys_path |
| 72 | |
| 73 | added_includes = [] |
| 74 | for f in input_api.AffectedFiles(): |
| 75 | if not CppChecker.IsCppFile(f.LocalPath()): |
| 76 | continue |
| 77 | |
| 78 | changed_lines = [line for line_num, line in f.ChangedContents()] |
| 79 | added_includes.append([f.LocalPath(), changed_lines]) |
| 80 | |
| 81 | deps_checker = checkdeps.DepsChecker(input_api.PresubmitLocalPath()) |
| 82 | |
| 83 | error_descriptions = [] |
| 84 | warning_descriptions = [] |
| 85 | for path, rule_type, rule_description in deps_checker.CheckAddedCppIncludes( |
| 86 | added_includes): |
| 87 | description_with_path = '%s\n %s' % (path, rule_description) |
| 88 | if rule_type == Rule.DISALLOW: |
| 89 | error_descriptions.append(description_with_path) |
| 90 | else: |
| 91 | warning_descriptions.append(description_with_path) |
| 92 | |
| 93 | results = [] |
| 94 | if error_descriptions: |
| 95 | results.append(output_api.PresubmitError( |
| 96 | 'You added one or more #includes that violate checkdeps rules.', |
| 97 | error_descriptions)) |
| 98 | if warning_descriptions: |
| 99 | results.append(output_api.PresubmitPromptOrNotify( |
| 100 | 'You added one or more #includes of files that are temporarily\n' |
| 101 | 'allowed but being removed. Can you avoid introducing the\n' |
| 102 | '#include? See relevant DEPS file(s) for details and contacts.', |
| 103 | warning_descriptions)) |
| 104 | return results |
| 105 | |
| 106 | |
dsinclair | 2ca2da5 | 2016-09-13 18:10:34 -0700 | [diff] [blame] | 107 | def _CheckIncludeOrderForScope(scope, input_api, file_path, changed_linenums): |
| 108 | """Checks that the lines in scope occur in the right order. |
| 109 | |
| 110 | 1. C system files in alphabetical order |
| 111 | 2. C++ system files in alphabetical order |
| 112 | 3. Project's .h files |
| 113 | """ |
| 114 | |
| 115 | c_system_include_pattern = input_api.re.compile(r'\s*#include <.*\.h>') |
| 116 | cpp_system_include_pattern = input_api.re.compile(r'\s*#include <.*>') |
| 117 | custom_include_pattern = input_api.re.compile(r'\s*#include ".*') |
| 118 | |
| 119 | C_SYSTEM_INCLUDES, CPP_SYSTEM_INCLUDES, CUSTOM_INCLUDES = range(3) |
| 120 | |
| 121 | state = C_SYSTEM_INCLUDES |
| 122 | |
| 123 | previous_line = '' |
| 124 | previous_line_num = 0 |
| 125 | problem_linenums = [] |
| 126 | out_of_order = " - line belongs before previous line" |
| 127 | for line_num, line in scope: |
| 128 | if c_system_include_pattern.match(line): |
| 129 | if state != C_SYSTEM_INCLUDES: |
| 130 | problem_linenums.append((line_num, previous_line_num, |
| 131 | " - C system include file in wrong block")) |
| 132 | elif previous_line and previous_line > line: |
| 133 | problem_linenums.append((line_num, previous_line_num, |
| 134 | out_of_order)) |
| 135 | elif cpp_system_include_pattern.match(line): |
| 136 | if state == C_SYSTEM_INCLUDES: |
| 137 | state = CPP_SYSTEM_INCLUDES |
| 138 | elif state == CUSTOM_INCLUDES: |
| 139 | problem_linenums.append((line_num, previous_line_num, |
| 140 | " - c++ system include file in wrong block")) |
| 141 | elif previous_line and previous_line > line: |
| 142 | problem_linenums.append((line_num, previous_line_num, out_of_order)) |
| 143 | elif custom_include_pattern.match(line): |
| 144 | if state != CUSTOM_INCLUDES: |
| 145 | state = CUSTOM_INCLUDES |
| 146 | elif previous_line and previous_line > line: |
| 147 | problem_linenums.append((line_num, previous_line_num, out_of_order)) |
| 148 | else: |
| 149 | problem_linenums.append((line_num, previous_line_num, |
| 150 | "Unknown include type")) |
| 151 | previous_line = line |
| 152 | previous_line_num = line_num |
| 153 | |
| 154 | warnings = [] |
| 155 | for (line_num, previous_line_num, failure_type) in problem_linenums: |
| 156 | if line_num in changed_linenums or previous_line_num in changed_linenums: |
| 157 | warnings.append(' %s:%d:%s' % (file_path, line_num, failure_type)) |
| 158 | return warnings |
| 159 | |
| 160 | |
| 161 | def _CheckIncludeOrderInFile(input_api, f, changed_linenums): |
| 162 | """Checks the #include order for the given file f.""" |
| 163 | |
| 164 | system_include_pattern = input_api.re.compile(r'\s*#include \<.*') |
| 165 | # Exclude the following includes from the check: |
| 166 | # 1) #include <.../...>, e.g., <sys/...> includes often need to appear in a |
| 167 | # specific order. |
| 168 | # 2) <atlbase.h>, "build/build_config.h" |
| 169 | excluded_include_pattern = input_api.re.compile( |
| 170 | r'\s*#include (\<.*/.*|\<atlbase\.h\>|"build/build_config.h")') |
| 171 | custom_include_pattern = input_api.re.compile(r'\s*#include "(?P<FILE>.*)"') |
| 172 | # Match the final or penultimate token if it is xxxtest so we can ignore it |
| 173 | # when considering the special first include. |
| 174 | test_file_tag_pattern = input_api.re.compile( |
| 175 | r'_[a-z]+test(?=(_[a-zA-Z0-9]+)?\.)') |
| 176 | if_pattern = input_api.re.compile( |
| 177 | r'\s*#\s*(if|elif|else|endif|define|undef).*') |
| 178 | # Some files need specialized order of includes; exclude such files from this |
| 179 | # check. |
| 180 | uncheckable_includes_pattern = input_api.re.compile( |
| 181 | r'\s*#include ' |
| 182 | '("ipc/.*macros\.h"|<windows\.h>|".*gl.*autogen.h")\s*') |
| 183 | |
| 184 | contents = f.NewContents() |
| 185 | warnings = [] |
| 186 | line_num = 0 |
| 187 | |
| 188 | # Handle the special first include. If the first include file is |
| 189 | # some/path/file.h, the corresponding including file can be some/path/file.cc, |
| 190 | # some/other/path/file.cc, some/path/file_platform.cc, some/path/file-suffix.h |
| 191 | # etc. It's also possible that no special first include exists. |
| 192 | # If the included file is some/path/file_platform.h the including file could |
| 193 | # also be some/path/file_xxxtest_platform.h. |
| 194 | including_file_base_name = test_file_tag_pattern.sub( |
| 195 | '', input_api.os_path.basename(f.LocalPath())) |
| 196 | |
| 197 | for line in contents: |
| 198 | line_num += 1 |
| 199 | if system_include_pattern.match(line): |
| 200 | # No special first include -> process the line again along with normal |
| 201 | # includes. |
| 202 | line_num -= 1 |
| 203 | break |
| 204 | match = custom_include_pattern.match(line) |
| 205 | if match: |
| 206 | match_dict = match.groupdict() |
| 207 | header_basename = test_file_tag_pattern.sub( |
| 208 | '', input_api.os_path.basename(match_dict['FILE'])).replace('.h', '') |
| 209 | |
| 210 | if header_basename not in including_file_base_name: |
| 211 | # No special first include -> process the line again along with normal |
| 212 | # includes. |
| 213 | line_num -= 1 |
| 214 | break |
| 215 | |
| 216 | # Split into scopes: Each region between #if and #endif is its own scope. |
| 217 | scopes = [] |
| 218 | current_scope = [] |
| 219 | for line in contents[line_num:]: |
| 220 | line_num += 1 |
| 221 | if uncheckable_includes_pattern.match(line): |
| 222 | continue |
| 223 | if if_pattern.match(line): |
| 224 | scopes.append(current_scope) |
| 225 | current_scope = [] |
| 226 | elif ((system_include_pattern.match(line) or |
| 227 | custom_include_pattern.match(line)) and |
| 228 | not excluded_include_pattern.match(line)): |
| 229 | current_scope.append((line_num, line)) |
| 230 | scopes.append(current_scope) |
| 231 | |
| 232 | for scope in scopes: |
| 233 | warnings.extend(_CheckIncludeOrderForScope(scope, input_api, f.LocalPath(), |
| 234 | changed_linenums)) |
| 235 | return warnings |
| 236 | |
| 237 | |
| 238 | def _CheckIncludeOrder(input_api, output_api): |
| 239 | """Checks that the #include order is correct. |
| 240 | |
| 241 | 1. The corresponding header for source files. |
| 242 | 2. C system files in alphabetical order |
| 243 | 3. C++ system files in alphabetical order |
| 244 | 4. Project's .h files in alphabetical order |
| 245 | |
| 246 | Each region separated by #if, #elif, #else, #endif, #define and #undef follows |
| 247 | these rules separately. |
| 248 | """ |
| 249 | def FileFilterIncludeOrder(affected_file): |
| 250 | black_list = (input_api.DEFAULT_BLACK_LIST) |
| 251 | return input_api.FilterSourceFile(affected_file, black_list=black_list) |
| 252 | |
| 253 | warnings = [] |
| 254 | for f in input_api.AffectedFiles(file_filter=FileFilterIncludeOrder): |
dsinclair | d33c8e3 | 2016-11-21 13:31:16 -0800 | [diff] [blame] | 255 | if f.LocalPath().endswith(('.cc', '.cpp', '.h', '.mm')): |
dsinclair | 2ca2da5 | 2016-09-13 18:10:34 -0700 | [diff] [blame] | 256 | changed_linenums = set(line_num for line_num, _ in f.ChangedContents()) |
| 257 | warnings.extend(_CheckIncludeOrderInFile(input_api, f, changed_linenums)) |
| 258 | |
| 259 | results = [] |
| 260 | if warnings: |
| 261 | results.append(output_api.PresubmitPromptOrNotify(_INCLUDE_ORDER_WARNING, |
| 262 | warnings)) |
| 263 | return results |
| 264 | |
Nicolas Pena | d824a90 | 2017-05-19 15:59:49 -0400 | [diff] [blame] | 265 | def _CheckTestDuplicates(input_api, output_api): |
| 266 | """Checks that pixel and javascript tests don't contain duplicates. |
| 267 | We use .in and .pdf files, having both can cause race conditions on the bots, |
| 268 | which run the tests in parallel. |
| 269 | """ |
| 270 | tests_added = [] |
| 271 | results = [] |
| 272 | for f in input_api.AffectedFiles(): |
| 273 | if not f.LocalPath().startswith(('testing/resources/pixel/', |
| 274 | 'testing/resources/javascript/')): |
| 275 | continue |
| 276 | end_len = 0 |
| 277 | if f.LocalPath().endswith('.in'): |
| 278 | end_len = 3 |
| 279 | elif f.LocalPath().endswith('.pdf'): |
| 280 | end_len = 4 |
| 281 | else: |
| 282 | continue |
Nicolas Pena | c4479c5 | 2017-06-26 11:45:06 -0400 | [diff] [blame] | 283 | path = f.LocalPath()[:-end_len] |
Nicolas Pena | d824a90 | 2017-05-19 15:59:49 -0400 | [diff] [blame] | 284 | if path in tests_added: |
| 285 | results.append(output_api.PresubmitError( |
| 286 | 'Remove %s to prevent shadowing %s' % (path + '.pdf', |
| 287 | path + '.in'))) |
| 288 | else: |
| 289 | tests_added.append(path) |
| 290 | return results |
| 291 | |
Nicolas Pena | c4479c5 | 2017-06-26 11:45:06 -0400 | [diff] [blame] | 292 | def _CheckPNGFormat(input_api, output_api): |
| 293 | """Checks that .png files have a format that will be considered valid by our |
| 294 | test runners. If a file ends with .png, then it must be of the form |
Nicolas Pena | d261b06 | 2017-06-26 16:54:43 -0400 | [diff] [blame] | 295 | NAME_expected(_(win|mac|linux))?.pdf.#.png""" |
Nicolas Pena | c4479c5 | 2017-06-26 11:45:06 -0400 | [diff] [blame] | 296 | expected_pattern = input_api.re.compile( |
Nicolas Pena | d261b06 | 2017-06-26 16:54:43 -0400 | [diff] [blame] | 297 | r'.+_expected(_(win|mac|linux))?\.pdf\.\d+.png') |
Nicolas Pena | c4479c5 | 2017-06-26 11:45:06 -0400 | [diff] [blame] | 298 | results = [] |
Nicolas Pena | d261b06 | 2017-06-26 16:54:43 -0400 | [diff] [blame] | 299 | for f in input_api.AffectedFiles(include_deletes=False): |
Nicolas Pena | c4479c5 | 2017-06-26 11:45:06 -0400 | [diff] [blame] | 300 | if not f.LocalPath().endswith('.png'): |
| 301 | continue |
| 302 | if expected_pattern.match(f.LocalPath()): |
| 303 | continue |
| 304 | results.append(output_api.PresubmitError( |
| 305 | 'PNG file %s does not have the correct format' % f.LocalPath())) |
| 306 | return results |
dsinclair | 2ca2da5 | 2016-09-13 18:10:34 -0700 | [diff] [blame] | 307 | |
Nico Weber | 077f1a3 | 2015-08-06 15:08:57 -0700 | [diff] [blame] | 308 | def CheckChangeOnUpload(input_api, output_api): |
| 309 | results = [] |
Dan Sinclair | 544bbc6 | 2016-03-14 15:07:39 -0400 | [diff] [blame] | 310 | results += _CheckUnwantedDependencies(input_api, output_api) |
Nico Weber | 077f1a3 | 2015-08-06 15:08:57 -0700 | [diff] [blame] | 311 | results += input_api.canned_checks.CheckPatchFormatted(input_api, output_api) |
Dan Sinclair | 22d6607 | 2016-02-22 11:56:05 -0500 | [diff] [blame] | 312 | results += input_api.canned_checks.CheckChangeLintsClean( |
| 313 | input_api, output_api, None, LINT_FILTERS) |
dsinclair | 2ca2da5 | 2016-09-13 18:10:34 -0700 | [diff] [blame] | 314 | results += _CheckIncludeOrder(input_api, output_api) |
Nicolas Pena | d824a90 | 2017-05-19 15:59:49 -0400 | [diff] [blame] | 315 | results += _CheckTestDuplicates(input_api, output_api) |
Nicolas Pena | c4479c5 | 2017-06-26 11:45:06 -0400 | [diff] [blame] | 316 | results += _CheckPNGFormat(input_api, output_api) |
Dan Sinclair | 544bbc6 | 2016-03-14 15:07:39 -0400 | [diff] [blame] | 317 | |
Nico Weber | 077f1a3 | 2015-08-06 15:08:57 -0700 | [diff] [blame] | 318 | return results |