Commit c2fecf47 authored by Vaclav Brozek's avatar Vaclav Brozek Committed by Commit Bot

Fix swapped messages in _CheckUniquePtr

_CheckUniquePtr consistes of two checks -- one related to nullptr, one
related to make_unique. The error messages are currently swapped by
accident. This CL fixes that and adds a check for that.

It also removes some confusing "java/src" subpaths from the mock file
paths in the check. Those got in via copy&paste and are confusing,
while not relevant for what the test is testing.

Bug: 827961
Change-Id: Iaf3272cfc5fe0a641feaee861ad17fe71505800b
Reviewed-on: https://chromium-review.googlesource.com/999604Reviewed-by: default avatarDirk Pranke <dpranke@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548807}
parent 6c0aed57
...@@ -1610,15 +1610,15 @@ def _CheckUniquePtr(input_api, output_api): ...@@ -1610,15 +1610,15 @@ def _CheckUniquePtr(input_api, output_api):
'%s:%d\n %s' % (local_path, line_number, line.strip())) '%s:%d\n %s' % (local_path, line_number, line.strip()))
errors = [] errors = []
if problems_constructor: if problems_nullptr:
errors.append(output_api.PresubmitError( errors.append(output_api.PresubmitError(
'The following files use std::unique_ptr<T>(). Use nullptr instead.', 'The following files use std::unique_ptr<T>(). Use nullptr instead.',
problems_constructor)) problems_nullptr))
if problems_nullptr: if problems_constructor:
errors.append(output_api.PresubmitError( errors.append(output_api.PresubmitError(
'The following files use explicit std::unique_ptr constructor.' 'The following files use explicit std::unique_ptr constructor.'
'Use std::make_unique<T>() instead.', 'Use std::make_unique<T>() instead.',
problems_nullptr)) problems_constructor))
return errors return errors
......
...@@ -1642,12 +1642,13 @@ class CheckUniquePtr(unittest.TestCase): ...@@ -1642,12 +1642,13 @@ class CheckUniquePtr(unittest.TestCase):
def testTruePositivesNullptr(self): def testTruePositivesNullptr(self):
mock_input_api = MockInputApi() mock_input_api = MockInputApi()
mock_input_api.files = [ mock_input_api.files = [
MockFile('dir/java/src/baz.cc', ['std::unique_ptr<T>()']), MockFile('dir/baz.cc', ['std::unique_ptr<T>()']),
MockFile('dir/java/src/baz-p.cc', ['std::unique_ptr<T<P>>()']), MockFile('dir/baz-p.cc', ['std::unique_ptr<T<P>>()']),
] ]
results = PRESUBMIT._CheckUniquePtr(mock_input_api, MockOutputApi()) results = PRESUBMIT._CheckUniquePtr(mock_input_api, MockOutputApi())
self.assertEqual(1, len(results)) self.assertEqual(1, len(results))
self.assertTrue('nullptr' in results[0].message)
self.assertEqual(2, len(results[0].items)) self.assertEqual(2, len(results[0].items))
self.assertTrue('baz.cc' in results[0].items[0]) self.assertTrue('baz.cc' in results[0].items[0])
self.assertTrue('baz-p.cc' in results[0].items[1]) self.assertTrue('baz-p.cc' in results[0].items[1])
...@@ -1655,17 +1656,17 @@ class CheckUniquePtr(unittest.TestCase): ...@@ -1655,17 +1656,17 @@ class CheckUniquePtr(unittest.TestCase):
def testTruePositivesConstructor(self): def testTruePositivesConstructor(self):
mock_input_api = MockInputApi() mock_input_api = MockInputApi()
mock_input_api.files = [ mock_input_api.files = [
MockFile('dir/java/src/foo.cc', ['return std::unique_ptr<T>(foo);']), MockFile('dir/foo.cc', ['return std::unique_ptr<T>(foo);']),
MockFile('dir/java/src/bar.mm', ['bar = std::unique_ptr<T>(foo)']), MockFile('dir/bar.mm', ['bar = std::unique_ptr<T>(foo)']),
MockFile('dir/java/src/mult.cc', [ MockFile('dir/mult.cc', [
'return', 'return',
' std::unique_ptr<T>(barVeryVeryLongFooSoThatItWouldNotFitAbove);' ' std::unique_ptr<T>(barVeryVeryLongFooSoThatItWouldNotFitAbove);'
]), ]),
MockFile('dir/java/src/mult2.cc', [ MockFile('dir/mult2.cc', [
'barVeryVeryLongLongBaaaaaarSoThatTheLineLimitIsAlmostReached =', 'barVeryVeryLongLongBaaaaaarSoThatTheLineLimitIsAlmostReached =',
' std::unique_ptr<T>(foo);' ' std::unique_ptr<T>(foo);'
]), ]),
MockFile('dir/java/src/mult3.cc', [ MockFile('dir/mult3.cc', [
'bar = std::unique_ptr<T>(', 'bar = std::unique_ptr<T>(',
' fooVeryVeryVeryLongStillGoingWellThisWillTakeAWhileFinallyThere);' ' fooVeryVeryVeryLongStillGoingWellThisWillTakeAWhileFinallyThere);'
]), ]),
...@@ -1673,6 +1674,7 @@ class CheckUniquePtr(unittest.TestCase): ...@@ -1673,6 +1674,7 @@ class CheckUniquePtr(unittest.TestCase):
results = PRESUBMIT._CheckUniquePtr(mock_input_api, MockOutputApi()) results = PRESUBMIT._CheckUniquePtr(mock_input_api, MockOutputApi())
self.assertEqual(1, len(results)) self.assertEqual(1, len(results))
self.assertTrue('std::make_unique' in results[0].message)
self.assertEqual(5, len(results[0].items)) self.assertEqual(5, len(results[0].items))
self.assertTrue('foo.cc' in results[0].items[0]) self.assertTrue('foo.cc' in results[0].items[0])
self.assertTrue('bar.mm' in results[0].items[1]) self.assertTrue('bar.mm' in results[0].items[1])
...@@ -1683,10 +1685,10 @@ class CheckUniquePtr(unittest.TestCase): ...@@ -1683,10 +1685,10 @@ class CheckUniquePtr(unittest.TestCase):
def testFalsePositives(self): def testFalsePositives(self):
mock_input_api = MockInputApi() mock_input_api = MockInputApi()
mock_input_api.files = [ mock_input_api.files = [
MockFile('dir/java/src/foo.cc', ['return std::unique_ptr<T[]>(foo);']), MockFile('dir/foo.cc', ['return std::unique_ptr<T[]>(foo);']),
MockFile('dir/java/src/bar.mm', ['bar = std::unique_ptr<T[]>(foo)']), MockFile('dir/bar.mm', ['bar = std::unique_ptr<T[]>(foo)']),
MockFile('dir/java/src/file.cc', ['std::unique_ptr<T> p = Foo();']), MockFile('dir/file.cc', ['std::unique_ptr<T> p = Foo();']),
MockFile('dir/java/src/baz.cc', [ MockFile('dir/baz.cc', [
'std::unique_ptr<T> result = std::make_unique<T>();' 'std::unique_ptr<T> result = std::make_unique<T>();'
]), ]),
] ]
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment