From 10912d9e7cf31d99c99b19c94f1f076f68354c89 Mon Sep 17 00:00:00 2001
From: dcheng <dcheng@chromium.org>
Date: Thu, 24 Nov 2016 08:34:14 -0800
Subject: [PATCH] Revert of Presubmit: Warn about useless forward declarations
 (patchset #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}
---
 PRESUBMIT.py      | 29 -------------------------
 PRESUBMIT_test.py | 55 -----------------------------------------------
 2 files changed, 84 deletions(-)

diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 33d75410d653d3..d78629a6d802be 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -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
@@ -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(
diff --git a/PRESUBMIT_test.py b/PRESUBMIT_test.py
index acb6adc48f4482..9bb6d47c73b81c 100755
--- a/PRESUBMIT_test.py
+++ b/PRESUBMIT_test.py
@@ -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()