From 8a87a46cc7f8d6cca5c0203ddba5d81149db7695 Mon Sep 17 00:00:00 2001 From: Igor Murashkin Date: Thu, 9 Nov 2017 13:48:29 -0800 Subject: [PATCH] CPPLINT.cfg root=setting is now relative to CFG file. Also further improve the documentation and testing around --root. Previously the root=setting in the CFG file was treated identically to passing --root=setting on the command line, which seems undesirable since it depends on were cpplint.cfg was invoked from (for relative paths). Judging on settings such as 'exclude_files' it seems within the spirit to make the settings 'current working directory' contextual to the same directory that CPPLINT.cfg is in. This also makes execution consistent (picking up the "correct" settings) regardless of the CWD when executing cpplint.py. Example: echo 'root=..' >> /a/b/c/CPPLINT.cfg cd / cpplint.py /a/b/c/source_file.h # expects header guard of C_SOURCE_FILE_H_ However the old behavior would use '/../' = '/' and incorrectly think the root was 'A_B_C_SOURCE_FILE_H_'. --- cpplint/cpplint.py | 34 +++++++++++++++++++++++----- cpplint/cpplint_unittest.py | 34 ++++++++++++++++++++++++++++ cpplint/nested/cpplint_test_header.h | 1 + 3 files changed, 63 insertions(+), 6 deletions(-) create mode 100644 cpplint/nested/cpplint_test_header.h diff --git a/cpplint/cpplint.py b/cpplint/cpplint.py index 7c624f2ad..a9815124f 100755 --- a/cpplint/cpplint.py +++ b/cpplint/cpplint.py @@ -114,12 +114,13 @@ ignored. Examples: - Assuming that src/.git exists, the header guard CPP variables for - src/chrome/browser/ui/browser.h are: + Assuming that top/src/.git exists (and cwd=top/src), the header guard + CPP variables for top/src/chrome/browser/ui/browser.h are: No flag => CHROME_BROWSER_UI_BROWSER_H_ --root=chrome => BROWSER_UI_BROWSER_H_ --root=chrome/browser => UI_BROWSER_H_ + --root=.. => SRC_CHROME_BROWSER_UI_BROWSER_H_ linelength=digits This is the allowed line length for the project. The default value is @@ -168,9 +169,9 @@ "linelength" allows to specify the allowed line length for the project. The "root" option is similar in function to the --root flag (see example - above). - - The "headers" option is similar in function to the --headers flag + above). Paths are relative to the directory of the CPPLINT.cfg. + + The "headers" option is similar in function to the --headers flag (see example above). CPPLINT.cfg has an effect on files in the same directory and all @@ -539,6 +540,7 @@ # The root directory used for deriving header guard CPP variable. # This is set by --root flag. _root = None +_root_debug = False # The allowed line length of files. # This is set by --linelength flag. @@ -1802,8 +1804,14 @@ def GetHeaderGuardCPPVariable(filename): file_path_from_root = fileinfo.RepositoryName() def FixupPathFromRoot(): + if _root_debug: + sys.stderr.write("\n_root fixup, _root = '%s', repository name = '%s'\n" + %(_root, fileinfo.RepositoryName())) + # Process the file path with the --root flag if it was set. if not _root: + if _root_debug: + sys.stderr.write("_root unspecified\n") return file_path_from_root def StripListPrefix(lst, prefix): @@ -1817,6 +1825,11 @@ def StripListPrefix(lst, prefix): # --root=subdir , lstrips subdir from the header guard maybe_path = StripListPrefix(PathSplitToList(file_path_from_root), PathSplitToList(_root)) + + if _root_debug: + sys.stderr.write("_root lstrip (maybe_path=%s, file_path_from_root=%s," + + " _root=%s)\n" %(maybe_path, file_path_from_root, _root)) + if maybe_path: return os.path.join(*maybe_path) @@ -1826,9 +1839,17 @@ def StripListPrefix(lst, prefix): maybe_path = StripListPrefix(PathSplitToList(full_path), PathSplitToList(root_abspath)) + + if _root_debug: + sys.stderr.write("_root prepend (maybe_path=%s, full_path=%s, " + + "root_abspath=%s)\n" %(maybe_path, full_path, root_abspath)) + if maybe_path: return os.path.join(*maybe_path) + if _root_debug: + sys.stderr.write("_root ignore, returning %s\n" %(file_path_from_root)) + # --root=FAKE_DIR is ignored return file_path_from_root @@ -5947,7 +5968,8 @@ def ProcessConfigOverrides(filename): sys.stderr.write('Line length must be numeric.') elif name == 'root': global _root - _root = val + # root directories are specified relative to CPPLINT.cfg dir. + _root = os.path.join(os.path.dirname(cfg_file), val) elif name == 'headers': ProcessHppHeadersOption(val) else: diff --git a/cpplint/cpplint_unittest.py b/cpplint/cpplint_unittest.py index 30f640e23..a1cd48166 100755 --- a/cpplint/cpplint_unittest.py +++ b/cpplint/cpplint_unittest.py @@ -4218,6 +4218,8 @@ def testBuildHeaderGuard(self): error_collector.ResultList()) def testBuildHeaderGuardWithRoot(self): + # note: Tested file paths must be real, otherwise + # the repository name lookup will fail. file_path = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'cpplint_test_header.h') file_info = cpplint.FileInfo(file_path) @@ -4238,9 +4240,33 @@ def testBuildHeaderGuardWithRoot(self): # # left-strip the header guard by using a root dir inside of the repo dir. + # relative directory cpplint._root = 'cpplint' self.assertEquals('CPPLINT_TEST_HEADER_H_', cpplint.GetHeaderGuardCPPVariable(file_path)) + + nested_file_path = os.path.join(os.path.dirname(os.path.abspath(__file__)), + os.path.join('nested', + 'cpplint_test_header.h')) + cpplint._root = os.path.join('cpplint', 'nested') + actual = cpplint.GetHeaderGuardCPPVariable(nested_file_path) + self.assertEquals('CPPLINT_TEST_HEADER_H_', + actual) + + # absolute directory + # (note that CPPLINT.cfg root=setting is always made absolute) + cpplint._root = os.path.join(os.path.dirname(os.path.abspath(__file__))) + self.assertEquals('CPPLINT_TEST_HEADER_H_', + cpplint.GetHeaderGuardCPPVariable(file_path)) + + nested_file_path = os.path.join(os.path.dirname(os.path.abspath(__file__)), + os.path.join('nested', + 'cpplint_test_header.h')) + cpplint._root = os.path.join(os.path.dirname(os.path.abspath(__file__)), + 'nested') + self.assertEquals('CPPLINT_TEST_HEADER_H_', + cpplint.GetHeaderGuardCPPVariable(nested_file_path)) + # --root flag is ignored if an non-existent directory is specified. cpplint._root = 'NON_EXISTENT_DIR' self.assertEquals('CPPLINT_CPPLINT_TEST_HEADER_H_', @@ -4250,6 +4276,7 @@ def testBuildHeaderGuardWithRoot(self): # than the repo dir # (using absolute paths) + # (note that CPPLINT.cfg root=setting is always made absolute) this_files_path = os.path.dirname(os.path.abspath(__file__)) (styleguide_path, this_files_dir) = os.path.split(this_files_path) (styleguide_parent_path, _) = os.path.split(styleguide_path) @@ -4260,6 +4287,10 @@ def testBuildHeaderGuardWithRoot(self): self.assertEquals('STYLEGUIDE_CPPLINT_CPPLINT_TEST_HEADER_H_', cpplint.GetHeaderGuardCPPVariable(file_path)) + # To run the 'relative path' tests, we must be in the directory of this test file. + cur_dir = os.getcwd() + os.chdir(this_files_path) + # (using relative paths) styleguide_rel_path = os.path.relpath(styleguide_path, this_files_path) # '..' @@ -4275,6 +4306,9 @@ def testBuildHeaderGuardWithRoot(self): cpplint._root = None + # Restore previous CWD. + os.chdir(cur_dir) + def testPathSplitToList(self): self.assertEquals([''], cpplint.PathSplitToList(os.path.join(''))) diff --git a/cpplint/nested/cpplint_test_header.h b/cpplint/nested/cpplint_test_header.h new file mode 100644 index 000000000..4307e8008 --- /dev/null +++ b/cpplint/nested/cpplint_test_header.h @@ -0,0 +1 @@ +// A test header for cpplint_unittest.py.