Doc and code fixes found via skylint. (#38)

* Fix up the skylint warning about depset usage.

* Remove the reassignment of max_bazel_version.

This was flagged by skylint, the reality is _is_at_most() does the parsing
so it shouldn't be parsed before sent on the way.

* Fix up the skylint warning about depset usage.

* Fixup the skylint warning about re-exports.

Just move to the explicit re-export form.

* Fix the fail() call to use the right variable.

skylint flagged this variable as undefined, but it it seems like this
was never right and should have been "key" all along.

* Fixup docstring formatting for skylint.

lib.bzl

@@ -14,17 +14,32 @@
 
 """Index from which multiple modules can be loaded."""
 
-load("//lib:collections.bzl", "collections")
-load("//lib:dicts.bzl", "dicts")
-load("//lib:new_sets.bzl", new_sets="sets")
-load("//lib:partial.bzl", "partial")
-load("//lib:paths.bzl", "paths")
-load("//lib:selects.bzl", "selects")
-load("//lib:sets.bzl", "sets")
-load("//lib:shell.bzl", "shell")
-load("//lib:structs.bzl", "structs")
-load("//lib:versions.bzl", "versions")
+load("//lib:collections.bzl", _collections="collections")
+load("//lib:dicts.bzl", _dicts="dicts")
+load("//lib:new_sets.bzl", _new_sets="sets")
+load("//lib:partial.bzl", _partial="partial")
+load("//lib:paths.bzl", _paths="paths")
+load("//lib:selects.bzl", _selects="selects")
+load("//lib:sets.bzl", _sets="sets")
+load("//lib:shell.bzl", _shell="shell")
+load("//lib:structs.bzl", _structs="structs")
+load("//lib:versions.bzl", _versions="versions")
 
 # The unittest module is treated differently to give more convenient names to
 # the assert functions, while keeping them in the same .bzl file.
-load("//lib:unittest.bzl", "asserts", "unittest")
+load("//lib:unittest.bzl", _asserts="asserts", _unittest="unittest")
+
+
+collections = _collections
+dicts = _dicts
+new_sets = _new_sets
+partial = _partial
+paths = _paths
+selects = _selects
+sets = _sets
+shell = _shell
+structs = _structs
+versions = _versions
+
+asserts = _asserts
+unittest = _unittest

lib/collections.bzl

@@ -21,6 +21,7 @@ def _after_each(separator, iterable):
   Args:
     separator: The value to insert after each item in `iterable`.
     iterable: The list into which to intersperse the separator.
+
   Returns:
     A new list with `separator` after each item in `iterable`.
   """
@@ -38,6 +39,7 @@ def _before_each(separator, iterable):
   Args:
     separator: The value to insert before each item in `iterable`.
     iterable: The list into which to intersperse the separator.
+
   Returns:
     A new list with `separator` before each item in `iterable`.
   """
@@ -56,6 +58,7 @@ def _uniq(iterable):
 
   Args:
     iterable: An iterable to filter.
+
   Returns:
     A new list with all unique elements from `iterable`.
   """

lib/dicts.bzl

@@ -28,6 +28,7 @@ def _add(*dictionaries):
 
   Args:
     *dictionaries: Zero or more dictionaries to be added.
+
   Returns:
     A new `dict` that has all the entries of the given dictionaries.
   """

lib/new_sets.bzl

@@ -50,13 +50,16 @@ def _copy(s):
   return struct(_values = dict(s._values))
 
 
-def _to_list(a):
+def _to_list(s):
   """Creates a list from the values in the set.
 
+  Args:
+    s: A set, as returned by `sets.make()`.
+
   Returns:
     A list of values inserted into the set.
   """
-  return a._values.keys()
+  return s._values.keys()
 
 
 def _insert(s, e):
@@ -106,11 +109,12 @@ def _contains(a, e):
 
 def _get_shorter_and_longer(a, b):
   """Returns two sets in the order of shortest and longest.
-  
+
   Args:
     a: A set, as returned by `sets.make()`.
     b: A set, as returned by `sets.make()`.
-   Returns:
+
+  Returns:
     `a`, `b` if `a` is shorter than `b` - or `b`, `a` if `b` is shorter than `a`.
   """
   if _length(a) < _length(b):
@@ -124,6 +128,7 @@ def _is_equal(a, b):
   Args:
     a: A set, as returned by `sets.make()`.
     b: A set, as returned by `sets.make()`.
+
   Returns:
     True if `a` is equal to `b`, False otherwise.
   """
@@ -207,11 +212,11 @@ def _difference(a, b):
 def _length(s):
   """Returns the number of elements in a set.
 
-    Args:
-      s: A set, as returned by `sets.make()`.
+  Args:
+    s: A set, as returned by `sets.make()`.
 
-    Returns:
-      An integer representing the number of elements in the set.
+  Returns:
+    An integer representing the number of elements in the set.
   """
   return len(s._values)
 

lib/paths.bzl

@@ -30,6 +30,7 @@ def _basename(p):
 
   Args:
     p: The path whose basename should be returned.
+
   Returns:
     The basename of the path, which includes the extension.
   """
@@ -45,6 +46,7 @@ def _dirname(p):
 
   Args:
     p: The path whose dirname should be returned.
+
   Returns:
     The dirname of the path.
   """
@@ -62,6 +64,7 @@ def _is_absolute(path):
 
   Args:
     path: A path (which is a string).
+
   Returns:
     `True` if `path` is an absolute path.
   """
@@ -82,6 +85,7 @@ def _join(path, *others):
   Args:
     path: A path segment.
     *others: Additional path segments.
+
   Returns:
     A string containing the joined paths.
   """
@@ -117,6 +121,7 @@ def _normalize(path):
 
   Args:
     path: A path.
+
   Returns:
     The normalized path.
   """
@@ -168,6 +173,7 @@ def _relativize(path, start):
   Args:
     path: The path to relativize.
     start: The ancestor path against which to relativize.
+
   Returns:
     The portion of `path` that is relative to `start`.
   """
@@ -199,6 +205,7 @@ def _replace_extension(p, new_extension):
     p: The path whose extension should be replaced.
     new_extension: The new extension for the file. The new extension should
         begin with a dot if you want the new filename to have one.
+
   Returns:
     The path with the extension replaced (or added, if it did not have one).
   """
@@ -213,6 +220,7 @@ def _split_extension(p):
 
   Args:
     p: The path whose root and extension should be split.
+
   Returns:
     A tuple `(root, ext)` such that the root is the path without the file
     extension, and `ext` is the file extension (which, if non-empty, contains

lib/selects.bzl

@@ -73,7 +73,7 @@ def _with_or_dict(input_dict):
         output_dict[config_setting] = value
     else:
       if key in output_dict.keys():
-        fail("key %s appears multiple times" % config_setting)
+        fail("key %s appears multiple times" % key)
       output_dict[key] = value
   return output_dict
 

lib/sets.bzl

@@ -116,10 +116,8 @@ def _union(*args):
     The set union of all sets or lists in `*args`.
   """
   _precondition_only_sets_or_lists(*args)
-  r = depset()
-  for a in args:
-    r += a
-  return r
+  args_deps = [depset(x) if type(x) == type([]) else x for x in args]
+  return depset(transitive=args_deps)
 
 
 def _difference(a, b):

lib/shell.bzl

@@ -28,6 +28,7 @@ def _array_literal(iterable):
   Args:
     iterable: A sequence of elements. Elements that are not strings will be
         converted to strings first, by calling `str()`.
+
   Returns:
     A string that represents the sequence as a shell array; that is,
     parentheses containing the quoted elements.
@@ -43,6 +44,7 @@ def _quote(s):
 
   Args:
     s: The string to quote.
+
   Returns:
     A quoted version of the string that can be passed to a shell command.
   """

lib/structs.bzl

@@ -20,6 +20,7 @@ def _to_dict(s):
 
   Args:
     s: A `struct`.
+
   Returns:
     A `dict` whose keys and values are the same as the fields in `s`. The
     transformation is only applied to the struct's fields and not to any

lib/unittest.bzl

@@ -53,6 +53,7 @@ def _make(impl, attrs=None):
     impl: The implementation function of the unit test.
     attrs: An optional dictionary to supplement the attrs passed to the
         unit test's `rule()` constructor.
+
   Returns:
     A rule definition that should be stored in a global whose name ends in
     `_test`.
@@ -145,6 +146,7 @@ def _begin(ctx):
   Args:
     ctx: The Skylark context. Pass the implementation function's `ctx` argument
         in verbatim.
+
   Returns:
     A test environment struct that must be passed to assertions and finally to
     `unittest.end`. Do not rely on internal details about the fields in this

lib/versions.bzl

@@ -108,7 +108,6 @@ def _check_bazel_version(minimum_bazel_version, maximum_bazel_version=None, baze
         bazel_version, minimum_bazel_version))
 
   if maximum_bazel_version:
-    max_bazel_version = _parse_bazel_version(maximum_bazel_version)
     if not _is_at_most(
         threshold = maximum_bazel_version,
         version = bazel_version):

skylark_library.bzl

@@ -24,9 +24,8 @@ SkylarkLibraryInfo = provider(
 )
 
 def _skylark_library_impl(ctx):
-  all_files = depset(ctx.files.srcs, order="postorder")
-  for dep in ctx.attr.deps:
-    all_files += dep.files
+  deps_files = [depset(x.files, order="postorder") for x in ctx.attr.deps]
+  all_files = depset(ctx.files.srcs, order="postorder", transitive=deps_files)
   return [
       # All dependent files should be listed in both `files` and in `runfiles`;
       # this ensures that a `skylark_library` can be referenced as `data` from