aboutsummaryrefslogtreecommitdiffhomepage
path: root/site/docs/skylark/skylint.md
blob: 06c1b83e7e4ce8a47f3eac0d98ab491c31ecea7f (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
---
layout: documentation
title: Skylint
---

# Skylint

[Style guide](https://docs.bazel.build/versions/master/skylark/bzl-style.html)

This document explains how to use Skylint, the Skylark linter.

<!-- [TOC] -->

## The linter CLI tool

### Building the linter

To build the linter from source, use the following commands:

```
$ git clone https://github.com/bazelbuild/bazel.git
$ cd bazel
$ bazel build //src/tools/skylark/java/com/google/devtools/skylark/skylint:Skylint
```

After that, the linter is available at `bazel-bin/src/tools/skylark/java/com/google/devtools/skylark/skylint/Skylint`.
You can copy it to somewhere else if you prefer.

### Running the linter

Assuming you have the linter binary available at `/path/to/Skylint` from the
previous step, you can run it like this:

```
$ /path/to/Skylint /path/to/bzl/file.bzl
```

where `/path/to/Skylint` is the path of the binary from the previous section.

## The checks in detail

This section explains which checks the linter performs and how to deal with
false positives.

### Deprecating functions (docstring format) [deprecated-symbol]

<a name="deprecated-symbol"></a>
To deprecate a function, add a `Deprecated:` section to the docstring, similarly
to a `Returns:` section.

```
def foo():
  """An example function.

  Deprecated:
    <reason and alternative>
  """
  # …

def bar():
  foo() # warning: "usage of 'foo' is deprecated: <reason and alternative>"
```

Note that the explanation starts on the next line after `Deprecated:` and may
occupy multiple lines, with all lines indented by two spaces.

### Using the operator + on dictionaries [deprecated-plus-dict]

<a name="deprecated-plus-dict"></a>
The `+` operator (and similarly `+=`) is deprecated for dictionaries. Instead,
use the following:

You can import a helper function from [Skylib](https://github.com/bazelbuild/bazel-skylib)
and use it like this:

```
load("@bazel_skylib//lib:dicts.bzl", "dicts")
dicts.add(d1, d2, d3) # instead of d1 + d2 + d3
```

### Using the operator + on depset [deprecated-plus-depset]

<a name="deprecated-plus-depset"></a>
The `+` operator (and similarly `+=`) is deprecated for depsets. Instead,
use the depset constructor.

See [documentation on depsets](depsets.md) for background and examples of use.

```
  d1 = depset(items1)
  d2 = depset(items2)
  combined = depset(transitive=[d1, d2])
```


### Docstrings

<a name="missing-module-docstring"></a>
<a name="missing-function-docstring"></a>
<a name="bad-docstring-format"></a>
Categories: [missing-module-docstring] [missing-function-docstring] [bad-docstring-format]

The Skylark conventions for docstrings are similar to the [the Python
conventions](https://google.github.io/styleguide/pyguide.html?showone=Comments#Comments).
Docstrings are triple-quoted string literals and the closing quotes have to
appear on their own line unless the docstring is only one line long.

A file-level docstring is the first statement of a file, even before the
`load()` statements. A function docstring is the first statement in the function
body.

Sections in the docstring (such as `Args:` and `Returns:`) are separated by a
blank line. Their contents are indented by two spaces.
Example:

```
"""This module contains some docstrings examples."""

def example_function1():
  """Illustrates the usage of a one-line function docstring."""

def example_function2(foo, bar):
  """Illustrates the format of a longer function docstring.

  After the one-line summary comes the description body.
  It contains additional information about the function.

  The description can span more than one paragraph.

  Args:
    foo: documentation of the first parameter.
      Subsequent lines have to be indented by two spaces.
    bar: documentation of the second parameter

  Returns:
    documentation of the return value

  Deprecated:
    This function is deprecated for <reason>. Use <alternative> instead.
  """
  return "baz"
```

#### Indentation

If the linter gives you confusing error messages, **check the indentation of the
docstring**. (The indentation rules are the same as [for
Python](https://google.github.io/styleguide/pyguide.html?showone=Comments#Comments).)
The analyzer tries to warn you about bad indentation but it may not catch all
edge cases. If the indentation is wrong, the analyzer may not recognize some
sections, such as `Args:`, which may then lead to further errors about missing
documentation for the parameters.

#### What to document [inconsistent-docstring]

<a name="inconsistent-docstring"></a>
The analyzer **requires docstrings** for:

*   each **module** (.bzl file)
*   each **public function** (i.e. a function not starting with an underscore)
    that contains at least 5 statements

If a **function has a multi-line docstring**, you also have to document (in
order):

*   all **parameters** of the function (in declaration order)
*   the **return value** if the function returns a value, i.e. it contains
    `return foo` instead of just `return`
*   a **deprecation warning** if the function is deprecated, cf. the
    [deprecation section](#deprecated-symbol) above

### Naming conventions

<a name="name-with-wrong-case"></a>
<a name="provider-name-suffix"></a>
<a name="confusing-name"></a>
Categories: [name-with-wrong-case] [provider-name-suffix] [confusing-name]

**TL;DR**: Most Skylark identifiers should be `snake_case`, not `camelCase`.

In detail, the rules are the following:

*   Use **`lower_snake_case`** for:
    *   **Functions**
    *   **Parameters**
    *   **Mutable** variables
*   Use **`UPPER_SNAKE_CASE`** for:
    *   **Constants** that are **immutable**. The variable must not be rebound
        and also its deep contents must not be changed.
*   Use **`UpperCamelCase`** for:
    *   **Providers**. Currently, the analyzer only allows assignments of the
        form `FooBarInfo = provider(...)`. In addition provider names have to
        end in the **suffix `Info`**.
*   **Never** use:
    *   **Builtin names** like `print`, `True`. Rebinding these is too
        confusing.
    *   **One-letter** names that are easy to confuse, namely **`O`, `l`, `I`**
        (easy to confuse with `0`, `I`, `l`, respectively).
    *   **Multiple underscores**: `__`, `___`, `____`, etc.
*   Use the **underscore `_`**:
    *   **only** to ignore the result of an assignment, as in `a, _ = tuple`.
    *   **never** to read the value of `_`, e.g. `f(_)`.

### Statements without effects [no-effect]

<a name="no-effect"></a>
If a statement is just an expression that is not a function call, the analyzer
warns `expression result not used`. Most likely, you forgot to do something with
that value. Examples: `1 + foo()`, `foo[bar]`.

#### List comprehensions

List comprehensions inside a function should be transformed to a for-loop.
This transformation is not possible at the top level because for-loops are only
allowed inside a function to encourage declarative code at the top level.
Therefore list comprehension at the top level are allowed but you should
consider whether it would be more readable to move them to a function. Example:

```
# This is allowed at the top level (but consider moving it to a function):
[do_something_with(foo) for foo in bar]

def baz():
  # This is BAD inside a function:
  [do_something_with(foo) for foo in bar]
  # Instead, use a for loop:
  for foo in bar:
    do_something_with(foo)
```

### Return value lint [missing-return-value]

<a name="missing-return-value"></a>
If a function returns with a value (`return foo`) in some execution paths and
without one (just `return` or reaching the end of a function) in other execution
paths, the analyzer will warn about this. The reason for this is that you
probably forgot to return the right value in some execution paths. If this is
not the case, you should make your intent clear by writing `return None`
instead.

#### Example

```
def foo():
  if ...:
    return False
  elif ...:
    # do stuff
    # forgot return statement here, which generates the warning
  else:
    return True
```

#### "I know the else-branch cannot happen"

Suppose you have code like this:

```
def foo():
  if cond1:
    return foo
  elif cond2:
    return bar
  # I know the else-branch can't happen but the analyzer complains
```

In such a case, just add `fail("unreachable")` or something along these lines
to the end of the function in order to silence the warning.

### Uninitialized variables [uninitialized-variable]

<a name="uninitialized-variable"></a>
If a variable is not initialized before it's used on every execution path, the
analyzer warns about it:

```
def foo():
  if cond1:
    foo = bar
  elif cond2:
    foo = baz
  print(foo) # warning: 'foo' may not have been initialized
```

Most likely, the author forgot to initialize `foo` in the `else` branch.
However, if this is a false positive because you know that the `else` branch can
never happen, add an `else`-branch with the line `fail("unreachable")` or
something similar. Then the analyzer won't complain.

If your code is more complex and it is impossible to determine statically that a
variable is initialized before usage, just initialize the variable with `None`
before using it.

### Unused bindings [unused-binding]

<a name="unused-binding"></a>
If a binding of an identifier is not used, the analyzer warns about it:

```
_PRIVATE_GLOBAL_UNUSED = 0 # warns because never used
PUBLIC_GLOBAL_UNUSED = 1 # doesn't warn because it might be exported

def public_function_unused(): # doesn't warn because it might be exported
  pass

# warns because unused function and parameter:
def _private_function_unused(param_unused):
  # warns because unused local variable → rename to '_' or  '_foo':
  for foo in range(0, 100):
    pass
```

The analyzer warns about unused identifiers except in the following cases:

*   If the identifier is global and public, there are no warnings because it may
    be `load()`ed from somewhere else.
*   If the identifier is `_`, there is no warning because the underscore signals
    that the a value is ignored.
*   If the identifier is a local variable and starts with an underscore, there
    is no warning because the underscore signals that the value is ignored (as
    opposed to global variables, where the underscore signifies that it is
    private).

#### Silencing the warning

If you want to **silence the warning**, you can do one of the following:

*   **Remove** the unused definition.
*   In case of a local variable, **rename** the variable to start with an
    underscore.
*   Look at the special cases below.

#### Unused parameters with fixed names

In case of a parameter whose name you cannot change (because you have to conform
to some API), you can use the following pattern:

```
def foo(param1, param2):
  _ignore = (param1, param2)
  # or:
  _ = (param1, param2)
  ...
```

This way, the parameters are used for the assignment of the ignored variable
`_ignore`. Hence the analyzer will not warn.

#### Re-exporting `load()`ed names

If you want to `load()` a name just to re-export it (and not use it in the
current file), use the following pattern.


```
# If you just want to re-export 'foo', DON'T do this:
load("bar.bzl", "foo")

# DO this instead:
load("bar.bzl", _foo="foo")
foo = _foo
```

This way, the name is still re-exported but doesn't generate a warning.

### Deprecated API

See [documentation](https://docs.bazel.build/versions/master/skylark/lib/ctx.html)
for more information.

### Miscellaneous lints

*   <a name="unreachable-statement"></a>**unreachable statements** [unreachable-statement]
*   <a name="load-at-top"></a>**Load statements** must be **at the top** of the file (after the docstring)
    [load-at-top]