Skip to content

Commit

Permalink
Extend presubmit _CheckUniquePtr to multiline
Browse files Browse the repository at this point in the history
The presubmit check _CheckUniquePtr guards against calling
std::unique_ptr constructor directly, instead directing code authors to
use std::make_unique.

This check currently fails to match multiline expressions. While it
catches
bar = std::unique_ptr<T>(foo);
it does not catch
bar =
    std::unique_ptr<T>(foo);
nor does it catch
bar = std::unique_ptr<T>(
    foo);

This CL fixes it by extending the match pattern to catch all lines with
the substring "std::unique_ptr<T>(". (But not those with
"std::unique_ptr<T>()", which should be handled by the "nullptr-check".)

Bug: 827961
Change-Id: I376b5e9811418205e294e97de0b6b7bcbf6891d2
Reviewed-on: https://chromium-review.googlesource.com/989735
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548045}
  • Loading branch information
Vaclav Brozek authored and Commit Bot committed Apr 4, 2018
1 parent 755a921 commit 95face6
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 7 deletions.
2 changes: 1 addition & 1 deletion PRESUBMIT.py
Original file line number Diff line number Diff line change
Expand Up @@ -1586,7 +1586,7 @@ def _CheckUniquePtr(input_api, output_api):
input_api.DEFAULT_BLACK_LIST),
white_list=(file_inclusion_pattern,))
return_construct_pattern = input_api.re.compile(
r'(=|\breturn)\s*std::unique_ptr<.*?(?<!])>\([^)]+\)')
r'(=|\breturn|^)\s*std::unique_ptr<.*?(?<!])>\(([^)]|$)')
null_construct_pattern = input_api.re.compile(
r'\b(?<!<)std::unique_ptr<[^>]*>([^(<]*>)?\(\)')
errors = []
Expand Down
22 changes: 16 additions & 6 deletions PRESUBMIT_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1646,21 +1646,31 @@ def testTruePositives(self):
MockFile('dir/java/src/bar.mm', ['bar = std::unique_ptr<T>(foo)']),
MockFile('dir/java/src/baz.cc', ['std::unique_ptr<T>()']),
MockFile('dir/java/src/baz-p.cc', ['std::unique_ptr<T<P>>()']),
# TODO(crbug.com/827961) Fix multi-line support.
#MockFile('dir/java/src/mult.cc', [
# 'barVeryVeryLongLongBaaaaaarSoThatTheLineLimitIsAlmostReached =',
# ' std::unique_ptr<T>(foo);'
#]),
MockFile('dir/java/src/mult.cc', [
'return',
' std::unique_ptr<T>(barVeryVeryLongFooSoThatItWouldNotFitAbove);'
]),
MockFile('dir/java/src/mult2.cc', [
'barVeryVeryLongLongBaaaaaarSoThatTheLineLimitIsAlmostReached =',
' std::unique_ptr<T>(foo);'
]),
MockFile('dir/java/src/mult3.cc', [
'bar = std::unique_ptr<T>(',
' fooVeryVeryVeryLongStillGoingWellThisWillTakeAWhileFinallyThere);'
]),
]

results = PRESUBMIT._CheckUniquePtr(mock_input_api, MockOutputApi())
# TODO(crbug.com/827961) Make the check return just one result, listing all
# affected files in it.
self.assertEqual(4, len(results))
self.assertEqual(7, len(results))
self.assertTrue('foo.cc' in results[0].message)
self.assertTrue('bar.mm' in results[1].message)
self.assertTrue('baz.cc' in results[2].message)
self.assertTrue('baz-p.cc' in results[3].message)
self.assertTrue('mult.cc' in results[4].message)
self.assertTrue('mult2.cc' in results[5].message)
self.assertTrue('mult3.cc' in results[6].message)

def testFalsePositives(self):
mock_input_api = MockInputApi()
Expand Down

0 comments on commit 95face6

Please sign in to comment.