Skip to content

Commit

Permalink
Revert of Presubmit: Warn about useless forward declarations (patchset
Browse files Browse the repository at this point in the history
…chromium#1 id:1 of https://codereview.chromium.org/2525263002/ )

Reason for revert:
Warning too aggressively + doesn't work with deleted files.

Original issue's description:
> Presubmit: Warn about useless forward declarations
>
> Checks that affected header files do not contain useless class
> or struct forward declaration.
>
> BUG=662195
> TEST=PRESUBMIT_test.py ForwardDeclarationTest
>
> Committed: https://crrev.com/1ae91afd6ab12165d8bd7981b9984fe7aec8d3d2
> Cr-Commit-Position: refs/heads/master@{#434329}

TBR=jochen@chromium.org,dpranke@google.com,phajdan.jr@chromium.org,asvitkine@chromium.org,jbriance@cisco.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=662195

Review-Url: https://codereview.chromium.org/2532563002
Cr-Commit-Position: refs/heads/master@{#434343}
  • Loading branch information
zetafunction authored and Commit bot committed Nov 24, 2016
1 parent b0445df commit 10912d9
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 84 deletions.
29 changes: 0 additions & 29 deletions PRESUBMIT.py
Original file line number Diff line number Diff line change
Expand Up @@ -1584,34 +1584,6 @@ def _CheckMojoUsesNewWrapperTypes(input_api, output_api):
return []


def _CheckUselessForwardDeclarations(input_api, output_api):
"""Checks that affected header files do not contain useless class
or struct forward declaration.
"""
results = []
class_pattern = input_api.re.compile(r'^class\s+(\w+);$',
input_api.re.MULTILINE)
struct_pattern = input_api.re.compile(r'^struct\s+(\w+);$',
input_api.re.MULTILINE)
for f in input_api.AffectedFiles():
if not f.LocalPath().endswith('.h'):
continue

contents = input_api.ReadFile(f)
fwd_decls = input_api.re.findall(class_pattern, contents)
fwd_decls.extend(input_api.re.findall(struct_pattern, contents))

for decl in fwd_decls:
count = sum(1 for _ in input_api.re.finditer(
r'\b%s\b' % input_api.re.escape(decl), contents))
if count == 1:
results.append(output_api.PresubmitPromptWarning(
'%s: %s forward declaration seems to be useless' %
(f.LocalPath(), decl)))

return results


def _CheckAndroidToastUsage(input_api, output_api):
"""Checks that code uses org.chromium.ui.widget.Toast instead of
android.widget.Toast (Chromium Toast doesn't force hardware
Expand Down Expand Up @@ -2061,7 +2033,6 @@ def _CommonChecks(input_api, output_api):
results.extend(_CheckJavaStyle(input_api, output_api))
results.extend(_CheckIpcOwners(input_api, output_api))
results.extend(_CheckMojoUsesNewWrapperTypes(input_api, output_api))
results.extend(_CheckUselessForwardDeclarations(input_api, output_api))

if any('PRESUBMIT.py' == f.LocalPath() for f in input_api.AffectedFiles()):
results.extend(input_api.canned_checks.RunUnitTestsInDirectory(
Expand Down
55 changes: 0 additions & 55 deletions PRESUBMIT_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1114,60 +1114,5 @@ def testAllowInComment(self):
self.assertEqual(0, len(warnings))


class ForwardDeclarationTest(unittest.TestCase):
def testCheckHeadersOnly(self):
mock_input_api = MockInputApi()
mock_input_api.files = [
MockAffectedFile('somewhere/file.cc', [
'class DummyClass;'
])
]
warnings = PRESUBMIT._CheckUselessForwardDeclarations(mock_input_api,
MockOutputApi())
self.assertEqual(0, len(warnings))

def testNoNestedDeclaration(self):
mock_input_api = MockInputApi()
mock_input_api.files = [
MockAffectedFile('somewhere/header.h', [
'class SomeClass {'
' protected:'
' class NotAMatch;'
'};'
])
]
warnings = PRESUBMIT._CheckUselessForwardDeclarations(mock_input_api,
MockOutputApi())
self.assertEqual(0, len(warnings))

def testSubStrings(self):
mock_input_api = MockInputApi()
mock_input_api.files = [
MockAffectedFile('somewhere/header.h', [
'class NotUsefulClass;',
'struct SomeStruct;',
'UsefulClass *p1;',
'SomeStructPtr *p2;'
])
]
warnings = PRESUBMIT._CheckUselessForwardDeclarations(mock_input_api,
MockOutputApi())
self.assertEqual(2, len(warnings))

def testUselessForwardDeclaration(self):
mock_input_api = MockInputApi()
mock_input_api.files = [
MockAffectedFile('somewhere/header.h', [
'class DummyClass;',
'struct DummyStruct;',
'class UsefulClass;',
'std::unique_ptr<UsefulClass> p;'
]),
]
warnings = PRESUBMIT._CheckUselessForwardDeclarations(mock_input_api,
MockOutputApi())
self.assertEqual(2, len(warnings))


if __name__ == '__main__':
unittest.main()

0 comments on commit 10912d9

Please sign in to comment.