WARNINGS.md

# Buildifier warnings

Warning categories supported by buildifier's linter:

  * [`attr-cfg`](#attr-cfg)
  * [`attr-license`](#attr-license)
  * [`attr-non-empty`](#attr-non-empty)
  * [`attr-output-default`](#attr-output-default)
  * [`attr-single-file`](#attr-single-file)
  * [`build-args-kwargs`](#build-args-kwargs)
  * [`bzl-visibility`](#bzl-visibility)
  * [`confusing-name`](#confusing-name)
  * [`constant-glob`](#constant-glob)
  * [`ctx-actions`](#ctx-actions)
  * [`ctx-args`](#ctx-args)
  * [`depset-iteration`](#depset-iteration)
  * [`depset-union`](#depset-union)
  * [`dict-concatenation`](#dict-concatenation)
  * [`duplicated-name`](#duplicated-name)
  * [`filetype`](#filetype)
  * [`function-docstring`](#function-docstring)
  * [`function-docstring-header`](#function-docstring-header)
  * [`function-docstring-args`](#function-docstring-args)
  * [`function-docstring-return`](#function-docstring-return)
  * [`git-repository`](#git-repository)
  * [`http-archive`](#http-archive)
  * [`integer-division`](#integer-division)
  * [`keyword-positional-params`](#keyword-positional-params)
  * [`list-append`](#list-append)
  * [`load`](#load)
  * [`load-on-top`](#load-on-top)
  * [`module-docstring`](#module-docstring)
  * [`name-conventions`](#name-conventions)
  * [`native-android`](#native-android)
  * [`native-build`](#native-build)
  * [`native-cc`](#native-cc)
  * [`native-java`](#native-java)
  * [`native-package`](#native-package)
  * [`native-proto`](#native-proto)
  * [`native-py`](#native-py)
  * [`no-effect`](#no-effect)
  * [`out-of-order-load`](#out-of-order-load)
  * [`output-group`](#output-group)
  * [`overly-nested-depset`](#overly-nested-depset)
  * [`package-name`](#package-name)
  * [`package-on-top`](#package-on-top)
  * [`positional-args`](#positional-args)
  * [`print`](#print)
  * [`redefined-variable`](#redefined-variable)
  * [`repository-name`](#repository-name)
  * [`return-value`](#return-value)
  * [`rule-impl-return`](#rule-impl-return)
  * [`same-origin-load`](#same-origin-load)
  * [`string-escape`](#string-escape)
  * [`string-iteration`](#string-iteration)
  * [`uninitialized`](#uninitialized)
  * [`unreachable`](#unreachable)
  * [`unsorted-dict-items`](#unsorted-dict-items)
  * [`unused-variable`](#unused-variable)

### How to disable warnings

All warnings can be disabled / suppressed / ignored by adding a special comment `# buildifier: disable=<category_name>` to
the expression that causes the warning. Historically comments with `buildozer` instead of
`buildifier` are also supported, they are equivalent.

#### Examples

```python
# buildifier: disable=no-effect
"""
A multiline comment as a string literal.
Docstrings don't trigger the warning if they are first statements of a file or a function.
"""

if debug:
    print("Debug information:", foo)  # buildifier: disable=print
```

--------------------------------------------------------------------------------

## <a name="attr-cfg"></a>`cfg = "data"` for attr definitions has no effect

  * Category name: `attr-cfg`
  * Flag in Bazel: [`--incompatible_disallow_data_transition`](https://github.com/bazelbuild/bazel/issues/6153)
  * Automatic fix: yes

The [Configuration](https://docs.bazel.build/versions/master/skylark/rules.html#configurations)
`cfg = "data"` is deprecated and has no effect. Consider removing it.

--------------------------------------------------------------------------------

## <a name="attr-license"></a>`attr.license()` is deprecated and shouldn't be used

  * Category name: `attr-license`
  * Flag in Bazel: `--incompatible_no_attr_license`
  * Automatic fix: no

The `attr.license()` method is almost never used and being deprecated.

--------------------------------------------------------------------------------

## <a name="attr-non-empty"></a>`non_empty` attribute for attr definitions are deprecated

  * Category name: `attr-non-empty`
  * Flag in Bazel: `--incompatible_disable_deprecated_attr_params`
  * Automatic fix: yes

The `non_empty` [attribute](https://docs.bazel.build/versions/master/skylark/lib/attr.html)
for attr definitions is deprecated, please use `allow_empty` with an opposite value instead.

--------------------------------------------------------------------------------

## <a name="attr-output-default"></a>The `default` parameter for `attr.output()`is deprecated

  * Category name: `attr-output-default`
  * Flag in Bazel: [`--incompatible_no_output_attr_default`](https://github.com/bazelbuild/bazel/issues/7950)
  * Automatic fix: no

The `default` parameter of `attr.output()` is bug-prone, as two targets of the same rule would be
unable to exist in the same package under default behavior. Use Starlark macros to specify defaults
for these attributes instead.

--------------------------------------------------------------------------------

## <a name="attr-single-file"></a>`single_file` is deprecated

  * Category name: `attr-single-file`
  * Flag in Bazel: `--incompatible_disable_deprecated_attr_params`
  * Automatic fix: yes

The `single_file` [attribute](https://docs.bazel.build/versions/master/skylark/lib/attr.html)
is deprecated, please use `allow_single_file` instead.

--------------------------------------------------------------------------------

## <a name="build-args-kwargs"></a>`*args` and `**kwargs` are not allowed in BUILD files

  * Category name: `build-args-kwargs`
  * Flag in Bazel: `--incompatible_no_kwargs_in_build_files`
  * Automatic fix: no

Having `*args` or `**kwargs` makes BUILD files hard to read and manipulate. The list of
arguments should be explicit.

--------------------------------------------------------------------------------

## <a name="bzl-visibility"></a>Module shouldn't be used directly

  * Category name: `bzl-visibility`
  * Automatic fix: no

If a directory `foo` contains a subdirectory `internal` or `private`, only files located under `foo`
can access it.

For example, `dir/rules_mockascript/private/foo.bzl` can be loaded from
`dir/rules_mockascript/private/bar.bzl` or `dir/rules_mockascript/sub/public.bzl`,
but not from `dir/other_rule/file.bzl`.

--------------------------------------------------------------------------------

## <a name="confusing-name"></a>Never use `l`, `I`, or `O` as names

  * Category name: `confusing-name`
  * Automatic fix: no

The names `l`, `I`, or `O` can be easily confused with `I`, `l`, or `0` correspondingly.

--------------------------------------------------------------------------------

## <a name="constant-glob"></a>Glob pattern has no wildcard ('*')

  * Category name: `constant-glob`
  * Automatic fix: no

[Glob function](https://docs.bazel.build/versions/master/be/functions.html#glob)
is used to get a list of files from the depot. The patterns (the first argument)
typically include a wildcard (* character). A pattern without a wildcard is
often useless and sometimes harmful.

To fix the warning, move the string out of the glob:

```diff
- glob(["*.cc", "test.cpp"])
+ glob(["*.cc"]) + ["test.cpp"]
```

**There’s one important difference**: before the change, Bazel would silently
ignore test.cpp if file is missing; after the change, Bazel will throw an error
if file is missing.

If `test.cpp` doesn’t exist, the fix becomes:

```diff
- glob(["*.cc", "test.cpp"])
+ glob(["*.cc"])
```

which improves maintenance and readability.

If no pattern has a wildcard, just remove the glob. It will also improve build
performance (glob can be relatively slow):

```diff
- glob(["test.cpp"])
+ ["test.cpp"]
```

--------------------------------------------------------------------------------

## <a name="ctx-actions"></a>`ctx.{action_name}` is deprecated

  * Category name: `ctx-actions`
  * Flag in Bazel: [`--incompatible_new_actions_api`](https://github.com/bazelbuild/bazel/issues/5825)
  * Automatic fix: yes

The following [actions](https://docs.bazel.build/versions/master/skylark/lib/actions.html)
are deprecated, please use the new API:

  * [`ctx.new_file`](https://docs.bazel.build/versions/master/skylark/lib/ctx.html#new_file) → [`ctx.actions.declare_file`](https://docs.bazel.build/versions/master/skylark/lib/actions.html#declare_file)
  * `ctx.experimental_new_directory` → [`ctx.actions.declare_directory`](https://docs.bazel.build/versions/master/skylark/lib/actions.html#declare_directory)
  * [`ctx.file_action`](https://docs.bazel.build/versions/master/skylark/lib/ctx.html#file_action) → [`ctx.actions.write`](https://docs.bazel.build/versions/master/skylark/lib/actions.html#write)
  * [`ctx.action(command = "...")`](https://docs.bazel.build/versions/master/skylark/lib/ctx.html#action) → [`ctx.actions.run_shell`](https://docs.bazel.build/versions/master/skylark/lib/actions.html#run_shell)
  * [`ctx.action(executable = "...")`](https://docs.bazel.build/versions/master/skylark/lib/ctx.html#action) → [`ctx.actions.run`](https://docs.bazel.build/versions/master/skylark/lib/actions.html#run)
  * [`ctx.empty_action`](https://docs.bazel.build/versions/master/skylark/lib/ctx.html#empty_action) → [`ctx.actions.do_nothing`](https://docs.bazel.build/versions/master/skylark/lib/actions.html#do_nothing)
  * [`ctx.template_action`](https://docs.bazel.build/versions/master/skylark/lib/ctx.html#template_action) → [`ctx.actions.expand_template`](https://docs.bazel.build/versions/master/skylark/lib/actions.html#expand_template)

--------------------------------------------------------------------------------

## <a name="ctx-args"></a>`ctx.actions.args().add()` for multiple arguments is deprecated

  * Category name: `ctx-args`
  * Flag in Bazel: [`--incompatible_disallow_old_style_args_add`](https://github.com/bazelbuild/bazel/issues/5822)
  * Automatic fix: yes

It's deprecated to use the [`add`](https://docs.bazel.build/versions/master/skylark/lib/Args.html#add)
method of `ctx.actions.args()` to add a list (or a depset) of variables. Please use either
[`add_all`](https://docs.bazel.build/versions/master/skylark/lib/Args.html#add_all) or
[`add_joined`](https://docs.bazel.build/versions/master/skylark/lib/Args.html#add_joined),
depending on the desired behavior.

--------------------------------------------------------------------------------

## <a name="depset-iteration"></a>Depset iteration is deprecated

  * Category name: `depset-iteration`
  * Flag in Bazel: [`--incompatible_depset_is_not_iterable`](https://github.com/bazelbuild/bazel/issues/5816)
  * Automatic fix: yes

Depsets are complex structures, iterations over them and lookups require flattening them to
a list which may be a heavy operation. To make it more obvious it's now required to call
the `.to_list()` method on them in order to be able to iterate their items:

```python
deps = depset()
[x.path for x in deps]  # deprecated
[x.path for x in deps.to_list()]  # recommended
```

--------------------------------------------------------------------------------

## <a name="depset-union"></a>Depsets should be joined using the depset constructor

  * Category name: `depset-union`
  * Flag in Bazel: [`--incompatible_depset_union`](https://github.com/bazelbuild/bazel/issues/5817)
  * Automatic fix: no

The following ways to merge two depsets are deprecated:

```python
depset1 + depset2
depset1 | depset2
depset1.union(depset2)
```

Please use the [depset](https://docs.bazel.build/versions/master/skylark/lib/depset.html) constructor
instead:

```python
depset(transitive = [depset1, depset2])
```

When fixing this issue, make sure you
[understand depsets](https://docs.bazel.build/versions/master/skylark/depsets.html)
and try to
[reduce the number of calls to depset](https://docs.bazel.build/versions/master/skylark/performance.html#reduce-the-number-of-calls-to-depset).

--------------------------------------------------------------------------------

## <a name="dict-concatenation"></a>Dictionary concatenation is deprecated

  * Category name: `dict-concatenation`
  * Flag in Bazel: [`--incompatible_disallow_dict_plus`](https://github.com/bazelbuild/bazel/issues/6461)
  * Automatic fix: no

The `+` operator to concatenate dicts is deprecated. The operator used to create a new dict and
copy the data to it. There are several ways to avoid it, for example, instead of `d = d1 + d2 + d3`
you can use one of the following:

* Use [Skylib](https://github.com/bazelbuild/bazel-skylib):

    ```python
    load("@bazel_skylib//lib:dicts.bzl", "dicts")

    d = dicts.add(d1, d2, d3)
    ```

* The same if you don't want to use Skylib:

    ```python
    d = dict(d1.items() + d2.items() + d3.items())
    ```

* The same in several steps:

    ```python
    d = dict(d1)  # If you don't want `d1` to be mutated
    d.update(d2)
    d.update(d3)
    ```

--------------------------------------------------------------------------------

## <a name="duplicated-name"></a>A rule with name `foo` was already found on line

  * Category name: `duplicated-name`
  * Automatic fix: no

### Background

Each label in Bazel has a unique name, and Bazel doesn’t allow two rules to have
the same name. With macros, this may be accepted by Bazel (if each macro
generates different rules):

```python
my_first_macro(name = "foo")
my_other_macro(name = "foo")
```

Although the build may work, this code can be very confusing. It can confuse
users reading a BUILD file (if they look for the rule “foo”, they may read see
only one of the macros). It will also confuse tools that edit BUILD files.

### How to fix it

Just change the name attribute of one rule/macro.

--------------------------------------------------------------------------------

## <a name="filetype"></a>The `FileType` function is deprecated

  * Category name: `filetype`
  * Flag in Bazel: [`--incompatible_disallow_filetype`](https://github.com/bazelbuild/bazel/issues/5831)
  * Automatic fix: no

The function `FileType` is deprecated.
Instead of using it as an argument to the [`rule` function](https://docs.bazel.build/versions/master/skylark/lib/globals.html#rule)
just use a list of strings.

--------------------------------------------------------------------------------

## <a name="function-docstring"></a><a name="function-docstring-header"></a><a name="function-docstring-args"></a><a name="function-docstring-return"></a>Function docstring

  * Category names:
    * `function-docstring`
    * `function-docstring-header`
    * `function-docstring-args`
    * `function-docstring-return`
  * Automatic fix: no

Public functions should have docstrings describing functions and their signatures.
A docstring is a string literal (not a comment) which should be the first statement
of a function (it may follow comment lines). Function docstrings are expected to be
formatted in the following way:

```python
"""One-line summary: must be followed and may be preceded by a blank line.

Optional additional description like this.

If it's a function docstring and the function has more than one argument, the docstring has
to document these parameters as follows:

Args:
  parameter1: description of the first parameter. Each parameter line
    should be indented by one, preferably two, spaces (as here).
  parameter2: description of the second
    parameter that spans two lines. Each additional line should have a
    hanging indentation of at least one, preferably two, additional spaces (as here).
  another_parameter (unused, mutable): a parameter may be followed
    by additional attributes in parentheses

Returns:
  Description of the return value.
  Should be indented by at least one, preferably two spaces (as here)
  Can span multiple lines.
"""
```

Docstrings are required for all public functions with at least 5 statements. If a docstring exists
it should start with a one-line summary line followed by an empty line. If a docsrting is required
or it describe some of the arguments, it should describe all of them. If a docstring is required and
the function returns a value, it should be described.

--------------------------------------------------------------------------------

## <a name="git-repository"></a>Function `git_repository` is not global anymore

  * Category name: `git-repository`
  * Flag in Bazel: [`--incompatible_remove_native_git_repository`](https://github.com/bazelbuild/bazel/issues/6569)
  * Automatic fix: yes

Native `git_repository` and `new_git_repository` functions are removed.
Please use the Starlark versions instead:

```python
load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository", "new_git_repository")
```

--------------------------------------------------------------------------------

## <a name="http-archive"></a>Function `http_archive` is not global anymore

  * Category name: `http-archive`
  * Flag in Bazel: [`--incompatible_remove_native_http_archive`](https://github.com/bazelbuild/bazel/issues/6570)
  * Automatic fix: yes

Native `http_archive` function are removed.
Please use the Starlark versions instead:

```python
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
```

--------------------------------------------------------------------------------

## <a name="integer-division"></a>The `/` operator for integer division is deprecated

  * Category name: `integer-division`
  * Flag in Bazel: [`--incompatible_disallow_slash_operator`](https://github.com/bazelbuild/bazel/issues/5823)
  * Automatic fix: yes

The `/` operator is deprecated in favor of `//`, please use the latter for
integer division:

```python
a = b // c
d //= e
```

--------------------------------------------------------------------------------

## <a name="keyword-positional-params"></a>Keyword parameter should be positional

  * Category_name: `keyword-positional-params`
  * Automatic fix: yes

Some parameters for builtin functions in Starlark are keyword for legacy reasons;
their names are not meaningful (e.g. `x`). Making them positional-only will improve
the readability.

--------------------------------------------------------------------------------

## <a name="list-append"></a>Prefer using ".append()" to adding a single element list

  * Category name: `list-append`
  * Automatic fix: yes

Transforming `x += [expr]` to `x.append(expr)` avoids a list allocation.

--------------------------------------------------------------------------------

## <a name="load"></a>Loaded symbol is unused

  * Category name: `load`
  * Automatic fix: yes

### Background

[load](https://docs.bazel.build/versions/master/skylark/concepts.html#loading-an-extension)
is used to import definitions in a BUILD file. If the definition is not used in
the file, the load can be safely removed. If a symbol is loaded two times, you
will get a warning on the second occurrence.

### How to fix it

Delete the line. When load is used to import multiple symbols, you can remove
the unused symbols from the list. To fix your BUILD files automatically, try
this command:

```console
$ buildozer 'fix unusedLoads' path/to/BUILD
```

If you want to keep the load, you can disable the warning by adding a comment
`# @unused`.

--------------------------------------------------------------------------------

## <a name="load-on-top"></a>Load statements should be at the top of the file

  * Category name: `load-on-top`
  * Flag in Bazel: [`--incompatible_bzl_disallow_load_after_statement`](https://github.com/bazelbuild/bazel/issues/5815)
  * Automatic fix: yes

Load statements should be first statements (with the exception of `WORKSPACE` files),
they can follow only comments and docstrings.

--------------------------------------------------------------------------------

## <a name="module-docstring"></a>The file has no module docstring

  * Category name: `module-docstring`
  * Automatic fix: no

`.bzl` files should have docstrings on top of them. A docstring is a string literal
(not a comment) which should be the first statement of the file (it may follow
comment lines). For example:

```python
"""
This module contains build rules for my project.
"""

...
```

--------------------------------------------------------------------------------

## <a name="name-conventions"></a>Name conventions

  * Category name: `name-conventions`
  * Automatic fix: no

By convention, all variables should be lower_snake_case, constant should be
UPPER_SNAKE_CASE, and providers should be UpperCamelCase ending with `Info`.

--------------------------------------------------------------------------------

## <a name="native-android"></a>All Android build rules should be loaded from Starlark

  * Category name: `native-android`
  * Automatic fix: yes

The Android build rules should be loaded from Starlark. The native rules [will be
disabled](https://github.com/bazelbuild/bazel/issues/8391).

--------------------------------------------------------------------------------

## <a name="native-build"></a>The `native` module shouldn't be used in BUILD files

  * Category name: `native-build`
  * Automatic fix: yes

There's no need in using `native.` in BUILD files, its members are available as global symbols
there.

--------------------------------------------------------------------------------

## <a name="native-cc"></a>All C++ build rules should be loaded from Starlark

  * Category name: `native-cc`
  * Flag in Bazel: [`--incompatible_load_cc_rules_from_bzl`](https://github.com/bazelbuild/bazel/issues/8743)
  * Automatic fix: yes

The C++ build rules should be loaded from Starlark. The native rules [will be
disabled](https://github.com/bazelbuild/bazel/issues/8743).

--------------------------------------------------------------------------------

## <a name="native-java"></a>All Java build rules should be loaded from Starlark

  * Category name: `native-java`
  * Flag in Bazel: [`--incompatible_load_java_rules_from_bzl`](https://github.com/bazelbuild/bazel/issues/8746)
  * Automatic fix: yes

The Java build rules should be loaded from Starlark. The native rules [will be
disabled](https://github.com/bazelbuild/bazel/issues/8746).

--------------------------------------------------------------------------------

## <a name="native-package"></a>`native.package()` shouldn't be used in .bzl files

  * Category name: `native-package`
  * Automatic fix: no

It's discouraged and will be disallowed to use `native.package()` in .bzl files. It can silently
modify the semantics of a BUILD file and makes it hard to maintain.

--------------------------------------------------------------------------------

## <a name="native-proto"></a>All Proto build rules and symbols should be loaded from Starlark

  * Category name: `native-proto`
  * Flag in Bazel: [`--incompatible_load_proto_rules_from_bzl`](https://github.com/bazelbuild/bazel/issues/8922)
  * Automatic fix: yes

The Proto build rules and symbols should be loaded from Starlark. The native
rules [will be disabled](https://github.com/bazelbuild/bazel/issues/8922).

--------------------------------------------------------------------------------

## <a name="native-py"></a>All Python build rules should be loaded from Starlark

  * Category name: `native-py`
  * Flag in Bazel: [`--incompatible_load_python_rules_from_bzl`](https://github.com/bazelbuild/bazel/issues/9006)
  * Automatic fix: yes

The Python build rules should be loaded from Starlark. The native rules [will be
disabled](https://github.com/bazelbuild/bazel/issues/9006).

--------------------------------------------------------------------------------

## <a name="no-effect"></a>Expression result is not used

  * Category name: `no-effect`
  * Automatic fix: no

The statement has no effect. Consider removing it or storing its result in a
variable.

--------------------------------------------------------------------------------

## <a name="out-of-order-load"></a>Load statements should be ordered by their labels

  * Category name: `out-of-order-load`
  * Automatic fix: yes
  * [Disabled by default](buildifier/README.md#linter)

Load statements should be ordered by their first argument - extension file label.
This makes it easier to developers to locate loads of interest and reduces chances
for conflicts when performing large-scale automated refactoring.

When applying automated fixes, it's highly recommended to also use
[`load-on-top`](#load-on-top) fixes, since otherwise the relative order
of a symbol load and its usage can change resulting in runtime error.

--------------------------------------------------------------------------------

## <a name="output-group"></a>`ctx.attr.dep.output_group` is deprecated

  * Category name: `output-group`
  * Flag in Bazel: [`--incompatible_no_target_output_group`](https://github.com/bazelbuild/bazel/issues/7949)
  * Automatic fix: yes

The `output_group` field of a target is deprecated in favor of the
[`OutputGroupInfo` provider](https://docs.bazel.build/versions/master/skylark/lib/OutputGroupInfo.html).

--------------------------------------------------------------------------------

## <a name="overly-nested-depset"></a>The depset is potentially overly nested

  * Category name: `overly-nested-depset`
  * Automatic fix: no

If a depset is iteratively chained in a for loop, e.g. the following pattern is used:

```python
for ...:
    x = depset(..., transitive = [..., x, ...])
```

this can result in an overly nested depset with a long chain of transitive elements. Such patterns
can lead to performance problems, consider refactoring the code to create a flat list of transitive
elements and call the depset constructor just once:

```python
transitive = []

for ...:
    transitive += ...

x = depset(..., transitive = transitive)
```

Or in simple cases you can use list comprehensions instead:

```python
x = depset(..., transitive = [y.deps for y in ...])
```

For more information, read Bazel documentation about 
[depsets](https://docs.bazel.build/versions/master/skylark/depsets.html)
and
[reducing the number of calls to depset](https://docs.bazel.build/versions/master/skylark/performance.html#reduce-the-number-of-calls-to-depset).

--------------------------------------------------------------------------------

## <a name="package-name"></a>Global variable `PACKAGE_NAME` is deprecated

  * Category name: `package-name`
  * Flag in Bazel: [`--incompatible_package_name_is_a_function`](https://github.com/bazelbuild/bazel/issues/5827)
  * Automatic fix: yes

The global variable `PACKAGE_NAME` is deprecated, please use
[`native.package_name()`](https://docs.bazel.build/versions/master/skylark/lib/native.html#package_name)
instead.

--------------------------------------------------------------------------------

## <a name="package-on-top"></a>Package declaration should be at the top of the file

  * Category name: `package-on-top`
  * Automatic fix: no

Here is a typical structure of a BUILD file:

*   `load()` statements
*   `package()`
*   calls to rules, macros

Instantiating a rule and setting the package defaults later can be very
confusing, and has been a source of bugs (tools and humans sometimes believe
package applies to everything in a BUILD file). This might become an error in
the future (but it requires large-scale changes in google3).

### What can be used before package()?

The linter allows the following to be before `package()`:

*   comments
*   `load()`
*   variable declarations
*   `package_group()`
*   `licenses()`

--------------------------------------------------------------------------------

## <a name="positional-args"></a>Keyword arguments should be used over positional arguments

  * Category name: `positional-args`
  * Automatic fix: no

All top level calls (except for some built-ins) should use keyword args over
positional arguments. Positional arguments can cause subtle errors if the order
is switched or if an argument is removed. Keyword args also greatly improve
readability.

```diff
- my_macro("foo", "bar")
+ my_macro(name = "foo", env = "bar")
```

The linter allows the following functions to be called with positional
arguments:

*   `load()`
*   `vardef()`
*   `export_files()`
*   `licenses()`
*   `print()`

--------------------------------------------------------------------------------

## <a name="print"></a>`print()` is a debug function and shouldn't be submitted

  * Category name: `print`
  * Automatic fix: no

Using the `print()` function for warnings is discouraged: they are often spammy and
non actionable, the people who see the warning are usually not the people who can
fix the code to make the warning disappear, and the actual maintainers of the code
may never see the warning.

--------------------------------------------------------------------------------

## <a name="redefined-variable"></a>Variable has already been defined

  * Category name: `redefined-variable`
  * Automatic fix: no

### Background

In .bzl files, redefining a global variable is already forbidden. This helps
both humans and tools reason about the code. For consistency, we want to bring
this restriction also to BUILD files.

### How to fix it

Rename one of the variables.

Note that the content of lists and dictionaries can still be modified. We will
forbid reassignment, but not every side-effect.

--------------------------------------------------------------------------------

## <a name="repository-name"></a>Global variable `REPOSITORY_NAME` is deprecated

  * Category name: `repository-name`
  * Flag in Bazel: [`--incompatible_package_name_is_a_function`](https://github.com/bazelbuild/bazel/issues/5827)
  * Automatic fix: yes

The global variable `REPOSITORY_NAME` is deprecated, please use
[`native.repository_name()`](https://docs.bazel.build/versions/master/skylark/lib/native.html#repository_name)
instead.

--------------------------------------------------------------------------------

## <a name="return-value"></a>Some but not all execution paths of a function return a value

  * Category name: `return-value`
  * Automatic fix: no

Some but not all execution paths of a function return a value. Either there's
an explicit empty `return` statement, or an implcit return in the end of a
function. If it is intentional, make it explicit using `return None`. If you
know certain parts of the code cannot be reached, add the statement
`fail("unreachable")` to them.

--------------------------------------------------------------------------------

## <a name="rule-impl-return"></a>Avoid using the legacy provider syntax

  * Category name: `rule-impl-return`
  * Automatic fix: no

Returning structs from rule implementation functions is
<a href="https://docs.bazel.build/versions/master/skylark/rules.html#migrating-from-legacy-providers">deprecated</a>,
consider using
<a href="https://docs.bazel.build/versions/master/skylark/rules.html#providers">providers</a>
or lists of providers instead.

--------------------------------------------------------------------------------

## <a name="same-origin-load"></a>Same label is used for multiple loads

  * Category name: `same-origin-load`
  * Automatic fix: yes

### Background

[load](https://docs.bazel.build/versions/master/skylark/concepts.html#loading-an-extension)
is used to import definitions in a BUILD file. If the same label is used for loading
symbols more the ones, all such loads can be merged into a single one.

### How to fix it

Merge all loads into a single one. For example,

```python
load(":f.bzl", "s1")
load(":f.bzl", "s2")
```

can be written more compactly as

```python
load(":f.bzl", "s1", "s2")
```

--------------------------------------------------------------------------------

## <a name="string-escape"></a>Invalid escape sequence

  * Category name: `string-escape`
  * Flag in Bazel: [`--incompatible_restrict_string_escapes`](https://github.com/bazelbuild/bazel/issues/8380)
  * Automatic fix: yes

Unrecognized escape sequences in string literals (e.g. `"\a \b"` is error-prone and shouldn't
be used. If you need the backslash symbol, escape it explicitly: `"\\a \\b"`.

--------------------------------------------------------------------------------

## <a name="string-iteration"></a>String iteration is deprecated

  * Category name: `string-iteration`
  * Flag in Bazel: [`--incompatible_string_is_not_iterable`](https://github.com/bazelbuild/bazel/issues/5830)
  * Automatic fix: no

Iteration over strings often leads to confusion with iteration over a sequence of strings,
therefore strings won't be recognized as sequences of 1-element strings (like in Python).
Use string indexing and `len` instead:

```python
my_string = "hello world"
for i in range(len(my_string)):
    char = my_string[i]
    # do something with char
```

--------------------------------------------------------------------------------

## <a name="uninitialized"></a>Variable may not have been initialized

  * Category name: `uninitialized`
  * Automatic fix: no

The local value can be not initialized at the time of execution. It may happen if it's
initialized in one of the if-else clauses but not in all of them, or in a for-loop which
can potentially be empty.

--------------------------------------------------------------------------------

## <a name="unreachable"></a>The statement is unreachable

  * Category name: `unreachable`
  * Automatic fix: no

The statement is unreachable because it follows a `return`, `break`, `continue`,
or `fail()` statement.

--------------------------------------------------------------------------------

## <a name="unsorted-dict-items"></a>Dictionary items should be ordered by their keys

  * Category name: `unsorted-dict-items`
  * Automatic fix: yes
  * [Disabled by default](buildifier/README.md#linter)

Dictionary items should be sorted lexicagraphically by their keys. This makes
it easier to find the item of interest and reduces chances of conflicts when
performing large-scale automated refactoring.

The order is affected by `NamePriority` dictionary passed using `-tables` or
`-add_tables` flags.

If you want to preserve the original dictionary items order, you can disable
the warning by adding a comment `# @unsorted-dict-items` to the dictionary
expression or any of its enclosing expressins (binary, if etc). For example,

```python
# @unsorted-dict-items
d = {
  "b": "bvalue",
  "a": "avalue",
}
```

will not be reported as an issue because the assignment operation that uses
the dictionary with unsorted items has a comment disabling this warning.

--------------------------------------------------------------------------------

## <a name="unused-variable"></a>Variable is unused

  * Category name: `unused-variable`
  * Automatic fix: no

This happens when a variable is set but not used in the file, e.g.

```python
x = [1, 2]
```

The line can often be safely removed.

If you want to keep the variable, you can disable the warning by adding a
comment `# @unused`.

```python
x = [1, 2] # @unused
```