Skip to content

Commit

Permalink
CPPLINT.cfg root=setting is now relative to CFG file.
Browse files Browse the repository at this point in the history
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_'.
  • Loading branch information
iam committed Nov 10, 2017
1 parent e7ddd2a commit 8a87a46
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 6 deletions.
34 changes: 28 additions & 6 deletions cpplint/cpplint.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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):
Expand All @@ -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)

Expand All @@ -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

Expand Down Expand Up @@ -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:
Expand Down
34 changes: 34 additions & 0 deletions cpplint/cpplint_unittest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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_',
Expand All @@ -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)
Expand All @@ -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)
# '..'
Expand All @@ -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('')))
Expand Down
1 change: 1 addition & 0 deletions cpplint/nested/cpplint_test_header.h
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// A test header for cpplint_unittest.py.

0 comments on commit 8a87a46

Please sign in to comment.