Skip to content

Latest commit

 

History

History
643 lines (433 loc) · 24 KB

WARNINGS.md

File metadata and controls

643 lines (433 loc) · 24 KB
Raw

Buildifier warnings

Function call arguments should be in the following order

  • Category_name: args-order
  • Flag in Bazel: --incompatible_strict_argument_ordering
  • Automatic fix: yes

Function call arguments should be in the following order:

  • Positional arguments
  • Keyword arguments
  • Optional *arg
  • Optional **kwarg

cfg = "data" for attr definitions has no effect

The Configuration `cfg = "data" is deprecated and has no effect. Consider removing it.

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.

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 for attr definitions is deprecated, please use allow_empty with an opposite value instead.

The default parameter for attr.output()is deprecated

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.

single_file is deprecated

  • Category_name: attr-single-file
  • Flag in Bazel: --incompatible_disable_deprecated_attr_params
  • Automatic fix: yes

The single_file attribute is deprecated, please use allow_single_file instead.

Glob pattern has no wildcard ('*')

  • Category name: constant-glob
  • Automatic fix: no

Glob function 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:

- 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:

- 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):

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

How to disable this warning

You can disable this warning by adding # buildozer: disable=constant-glob on the line or at the beginning of a rule.

ctx.{action_name} is deprecated

The following actions are deprecated, please use the new API:

ctx.actions.args().add() for multiple arguments is deprecated

It's deprecated to use the add method of ctx.actions.args() to add a list (or a depset) of variables. Please use either add_all or add_joined, depending on the desired behavior.

Depset iteration is deprecated

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:

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

Depsets should be joined using the depset constructor

The following ways to merge two depsets are deprecated:

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

Please use the depset constructor instead:

depset(transitive = [depset1, depset2])

Dictionary concatenation is deprecated

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:

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

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

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

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

  • The same in several steps:

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

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):

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.

How to disable this warning

You can disable this warning by adding # buildozer: disable=duplicated-name on the line or at the beginning of a rule.

The FileType function is deprecated

The function FileType is deprecated. Instead of using it as an argument to the rule function just use a list of strings.

Function git_repository is not global anymore

Native git_repository and new_git_repository functions are being removed. Please use the Starklark versions instead:

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

Function http_archive is not global anymore

Native http_archive function are being removed. Please use the Starklark versions instead:

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

The / operator for integer division is deprecated

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

a = b // c d //= e

Loaded symbol is unused

  • Category_name: load
  • Automatic fix: yes

Background

load 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:

buildozer 'fix unusedLoads' path/to/BUILD

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

How to disable this warning

You can disable this warning by adding # buildozer: disable=load on the line or at the beginning of a rule.

Load statements should be at the top of the file.

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

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.

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.

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.

Load statements should be ordered by their labels.

  • Category_name: out-of-order-load
  • Automatic fix: yes

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 fixes, since otherwise the relative order of a symbol load and its usage can change resulting in runtime error.

Dictionary items should be ordered by their keys.

  • Category_name: unsorted-dict-items
  • Automatic fix: yes

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,

# @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.

ctx.attr.dep.output_group is deprecated

The output_group field of a target is deprecated in favor of the OutputGroupInfo provider.

Global variable PACKAGE_NAME is deprecated

The global variable PACKAGE_NAME is deprecated, please use native.package_name() instead.

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()

How to disable this warning

You can disable this warning by adding # buildozer: disable=package-on-top on the line or at the beginning of a rule.

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.

- 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()

How to disable this warning

You can disable this warning by adding # buildozer: disable=positional-args on the line or at the beginning of a rule.

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.

How to disable this warning

You can disable this warning by adding # buildozer: disable=unused-variable on the line or at the beginning of a rule.

Global variable REPOSITORY_NAME is deprecated

The global variable REPOSITORY_NAME is deprecated, please use native.repository_name() instead.

Same label is used for multiple loads

  • Category_name: same-origin-load
  • Automatic fix: yes

Background

load 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,

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

can be written more compactly as

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

String iteration is deprecated

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:

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

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.

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.

x = [1, 2] # @unused

How to disable this warning

You can disable this warning by adding # buildozer: disable=unused-variable on the line or at the beginning of a rule.