Clean up pylint warnings in filesystem_mock.py.

third_party/WebKit/Tools/Scripts/webkitpy/common/system/filesystem_mock.py

Index: third_party/WebKit/Tools/Scripts/webkitpy/common/system/filesystem_mock.py
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/common/system/filesystem_mock.py b/third_party/WebKit/Tools/Scripts/webkitpy/common/system/filesystem_mock.py
index 0341d484fa06438d8d8cb96ff74c0919c7238598..ba0da8d2bbda7353eb8de887655c292a06625810 100644
--- a/third_party/WebKit/Tools/Scripts/webkitpy/common/system/filesystem_mock.py
+++ b/third_party/WebKit/Tools/Scripts/webkitpy/common/system/filesystem_mock.py
@@ -32,21 +32,18 @@ import hashlib
 import os
 import re
 
-from webkitpy.common.system import path
-
 
 class MockFileSystem(object):
     sep = '/'
     pardir = '..'
 
     def __init__(self, files=None, dirs=None, cwd='/'):
-        """Initializes a "mock" filesystem that can be used to completely
-        stub out a filesystem.
+        """Initializes a "mock" filesystem that can be used to replace the
+        FileSystem class in tests.
 
         Args:
-            files: a dict of filenames -> file contents. A file contents
-                value of None is used to indicate that the file should
-                not exist.
+            files: A dictionary of filenames to file contents. A file contents
+                value of None indicates that the file does not exist.
         """
         self.files = files or {}
         self.executable_files = set()
@@ -56,11 +53,11 @@ class MockFileSystem(object):
         self.cwd = cwd
         self.dirs = set(dirs or [])
         self.dirs.add(cwd)
-        for f in self.files:
-            d = self.dirname(f)
-            while not d in self.dirs:
-                self.dirs.add(d)
-                d = self.dirname(d)
+        for file_path in self.files:
+            directory = self.dirname(file_path)
+            while directory not in self.dirs:
+                self.dirs.add(directory)
+                directory = self.dirname(directory)
 
     def clear_written_files(self):
         # This function can be used to track what is written between steps in a test.
@@ -70,7 +67,7 @@ class MockFileSystem(object):
         raise IOError(errno.ENOENT, path, os.strerror(errno.ENOENT))
 
     def _split(self, path):
-        # This is not quite a full implementation of os.path.split
+        # This is not quite a full implementation of os.path.split; see:
         # http://docs.python.org/library/os.path.html#os.path.split
         if self.sep in path:
             return path.rsplit(self.sep, 1)
@@ -127,8 +124,10 @@ class MockFileSystem(object):
     def exists(self, path):
         return self.isfile(path) or self.isdir(path)
 
-    def files_under(self, path, dirs_to_skip=[], file_filter=None):
-        def filter_all(fs, dirpath, basename):
+    def files_under(self, path, dirs_to_skip=None, file_filter=None):
+        dirs_to_skip = dirs_to_skip or []
+
+        def filter_all(fs, dirpath, basename):  # pylint: disable=unused-argument
             return True

[jeffcarp, 2016/11/15 19:10:01]

Would turning this into a lambda get rid of the lint error? I seem to remember
that being the case for the code we worked on last week...

 
         file_filter = file_filter or filter_all
@@ -169,7 +168,7 @@ class MockFileSystem(object):
         glob_string = glob_string.replace('\\/', '/')
         path_filter = lambda path: re.match(glob_string, path)
 
-        # We could use fnmatch.fnmatch, but that might not do the right thing on windows.
+        # We could use fnmatch.fnmatch, but that might not do the right thing on Windows.
         existing_files = [path for path, contents in self.files.items() if contents is not None]
         return filter(path_filter, existing_files) + filter(path_filter, self.dirs)
 
@@ -187,7 +186,7 @@ class MockFileSystem(object):
 
     def join(self, *comps):
         # This function is called a lot, so we optimize it; there are
-        # unittests to check that we match _slow_but_correct_join(), above.
+        # unit tests to check that we match _slow_but_correct_join(), above.
         path = ''
         sep = self.sep
         for comp in comps:
@@ -205,8 +204,8 @@ class MockFileSystem(object):
         return path
 
     def listdir(self, path):
-        _, dirs, files = list(self.walk(path))[0]
-        return dirs + files
+        _, directories, files = list(self.walk(path))[0]
+        return directories + files
 
     def walk(self, top):
         sep = self.sep
@@ -216,21 +215,21 @@ class MockFileSystem(object):
         if not top.endswith(sep):
             top += sep
 
-        dirs = []
+        directories = []
         files = []
-        for f in self.files:
-            if self.exists(f) and f.startswith(top):
-                remaining = f[len(top):]
+        for file_path in self.files:
+            if self.exists(file_path) and file_path.startswith(top):
+                remaining = file_path[len(top):]
                 if sep in remaining:
-                    dir = remaining[:remaining.index(sep)]
-                    if not dir in dirs:
-                        dirs.append(dir)
+                    directory = remaining[:remaining.index(sep)]
+                    if directory not in directories:
+                        directories.append(directory)
                 else:
                     files.append(remaining)
-        file_system_tuples = [(top[:-1], dirs, files)]
-        for dir in dirs:
-            dir = top + dir
-            tuples_from_subdirs = self.walk(dir)
+        file_system_tuples = [(top[:-1], directories, files)]
+        for directory in directories:
+            directory = top + directory
+            tuples_from_subdirs = self.walk(directory)
             file_system_tuples += tuples_from_subdirs
         return file_system_tuples
 
@@ -239,7 +238,7 @@ class MockFileSystem(object):
             return 0
         self._raise_not_found(path)
 
-    def _mktemp(self, suffix='', prefix='tmp', dir=None, **kwargs):
+    def _mktemp(self, suffix='', prefix='tmp', dir=None, **_):  # pylint: disable=redefined-builtin
         if dir is None:
             dir = self.sep + '__im_tmp'
         curno = self.current_tmpno
@@ -253,7 +252,7 @@ class MockFileSystem(object):
             def __init__(self, fs, **kwargs):
                 self._kwargs = kwargs
                 self._filesystem = fs
-                self._directory_path = fs._mktemp(**kwargs)
+                self._directory_path = fs._mktemp(**kwargs)  # pylint: disable=protected-access
                 fs.maybe_make_directory(self._directory_path)
 
             def __str__(self):
@@ -262,7 +261,7 @@ class MockFileSystem(object):
             def __enter__(self):
                 return self._directory_path
 
-            def __exit__(self, type, value, traceback):
+            def __exit__(self, exception_type, exception_value, traceback):
                 # Only self-delete if necessary.
 
                 # FIXME: Should we delete non-empty directories?
@@ -390,16 +389,19 @@ class MockFileSystem(object):
         self.files[path] = None
         self.written_files[path] = None
 
-    def rmtree(self, path):
-        path = self.normpath(path)
+    def rmtree(self, path_to_remove):
+        path_to_remove = self.normpath(path_to_remove)
+
+        for file_path in self.files:
+            # We need to add a trailing separator to path_to_remove to avoid matching
+            # cases like path_to_remove='/foo/b' and file_path='/foo/bar/baz'.
+            if file_path == path_to_remove or file_path.startswith(path_to_remove + self.sep):
+                self.files[file_path] = None
 
-        for f in self.files:
-            # We need to add a trailing separator to path to avoid matching
-            # cases like path='/foo/b' and f='/foo/bar/baz'.
-            if f == path or f.startswith(path + self.sep):
-                self.files[f] = None
+        def should_remove(directory):
+            return directory == path_to_remove or directory.startswith(path_to_remove + self.sep)
 
-        self.dirs = set(filter(lambda d: not (d == path or d.startswith(path + self.sep)), self.dirs))
+        self.dirs = {d for d in self.dirs if not should_remove(d)}
 
     def copytree(self, source, destination):
         source = self.normpath(source)
@@ -435,21 +437,21 @@ class WritableBinaryFileObject(object):
     def __enter__(self):
         return self
 
-    def __exit__(self, type, value, traceback):
+    def __exit__(self, exception_type, exception_value, traceback):
         self.close()
 
     def close(self):
         self.closed = True
 
-    def write(self, str):
-        self.fs.files[self.path] += str
+    def write(self, string):
+        self.fs.files[self.path] += string
         self.fs.written_files[self.path] = self.fs.files[self.path]
 
 
 class WritableTextFileObject(WritableBinaryFileObject):
 
-    def write(self, str):
-        WritableBinaryFileObject.write(self, str.encode('utf-8'))
+    def write(self, string):
+        WritableBinaryFileObject.write(self, string.encode('utf-8'))
 
 
 class ReadableBinaryFileObject(object):
@@ -464,17 +466,17 @@ class ReadableBinaryFileObject(object):
     def __enter__(self):
         return self
 
-    def __exit__(self, type, value, traceback):
+    def __exit__(self, exception_type, exception_value, traceback):
         self.close()
 
     def close(self):
         self.closed = True
 
-    def read(self, bytes=None):
-        if not bytes:
+    def read(self, num_bytes=None):
+        if not num_bytes:
             return self.data[self.offset:]
         start = self.offset
-        self.offset += bytes
+        self.offset += num_bytes
         return self.data[start:self.offset]
 
 
@@ -487,8 +489,8 @@ class ReadableTextFileObject(ReadableBinaryFileObject):
         self.data.close()
         super(ReadableTextFileObject, self).close()
 
-    def read(self, bytes=-1):
-        return self.data.read(bytes)
+    def read(self, num_bytes=-1):
+        return self.data.read(num_bytes)
 
     def readline(self, length=None):
         return self.data.readline(length)