|
| 1 | +This file offers some tips on the coding conventions for rustc. This |
| 2 | +chapter covers [formatting](#formatting), [coding for correctness](#cc), |
| 3 | +[using crates from crates.io](#cio), and some tips on |
| 4 | +[structuring your PR for easy review](#er). |
| 5 | + |
| 6 | +<a name=formatting> |
| 7 | + |
| 8 | +# Formatting and the tidy script |
| 9 | + |
| 10 | +rustc is slowly moving towards the [Rust standard coding style][fmt]; |
| 11 | +at the moment, however, it follows a rather more *chaotic* style. We |
| 12 | +do have some mandatory formatting conventions, which are automatically |
| 13 | +enforced by a script we affectionately call the "tidy" script. The |
| 14 | +tidy script runs automatically when you do `./x.py test`. |
| 15 | + |
| 16 | +[fmt]: https://github.com/rust-lang-nursery/fmt-rfcs |
| 17 | + |
| 18 | +<a name=copyright> |
| 19 | + |
| 20 | +### Copyright notice |
| 21 | + |
| 22 | +All files must begin with the following copyright notice: |
| 23 | + |
| 24 | +``` |
| 25 | +// Copyright 2012-2013 The Rust Project Developers. See the COPYRIGHT |
| 26 | +// file at the top-level directory of this distribution and at |
| 27 | +// http://rust-lang.org/COPYRIGHT. |
| 28 | +// |
| 29 | +// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or |
| 30 | +// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license |
| 31 | +// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your |
| 32 | +// option. This file may not be copied, modified, or distributed |
| 33 | +// except according to those terms. |
| 34 | +``` |
| 35 | + |
| 36 | +The year at the top is not meaningful: copyright protections are in |
| 37 | +fact automatic from the moment of authorship. We do not typically edit |
| 38 | +the years on existing files. When creating a new file, you can use the |
| 39 | +current year if you like, but you don't have to. |
| 40 | + |
| 41 | +## Line length |
| 42 | + |
| 43 | +Lines should be at most 100 characters. It's even better if you can |
| 44 | +keep things to 80. |
| 45 | + |
| 46 | +**Ignoring the line length limit.** Sometimes -- in particular for |
| 47 | +tests -- it can be necessary to exempt yourself from this limit. In |
| 48 | +that case, you can add a comment towards the top of the file (after |
| 49 | +the copyright notice) like so: |
| 50 | + |
| 51 | +``` |
| 52 | +// ignore-tidy-linelength |
| 53 | +``` |
| 54 | + |
| 55 | +## Tabs vs spaces |
| 56 | + |
| 57 | +Prefer 4-space indent. |
| 58 | + |
| 59 | +<a name=cc> |
| 60 | + |
| 61 | +# Coding for correctness |
| 62 | + |
| 63 | +Beyond formatting, there are a few other tips that are worth |
| 64 | +following. |
| 65 | + |
| 66 | +## Prefer exhaustive matches |
| 67 | + |
| 68 | +Using `_` in a match is convenient, but it means that when new |
| 69 | +variants are added to the enum, they may not get handled correctly. |
| 70 | +Ask yourself: if a new variant were added to this enum, what's the |
| 71 | +chance that it would want to use the `_` code, versus having some |
| 72 | +other treatment? Unless the answer is "low", then prefer an |
| 73 | +exhaustive match. (The same advice applies to `if let` and `while |
| 74 | +let`, which are effectively tests for a single variant.) |
| 75 | + |
| 76 | +## Use "TODO" comments for things you don't want to forget |
| 77 | + |
| 78 | +As a useful tool to yourself, you can insert a `// TODO` comment |
| 79 | +for something that you want to get back to before you land your PR: |
| 80 | + |
| 81 | +```rust,ignore |
| 82 | +fn do_something() { |
| 83 | + if something_else { |
| 84 | + unimplemented!(): // TODO write this |
| 85 | + } |
| 86 | +} |
| 87 | +``` |
| 88 | + |
| 89 | +The tidy script will report an error for a `// TODO` comment, so this |
| 90 | +code would not be able to land until the TODO is fixed (or removed). |
| 91 | + |
| 92 | +This can also be useful in a PR as a way to signal from one commit that you are |
| 93 | +leaving a bug that a later commit will fix: |
| 94 | + |
| 95 | +```rust,ignore |
| 96 | +if foo { |
| 97 | + return true; // TODO wrong, but will be fixed in a later commit |
| 98 | +} |
| 99 | +``` |
| 100 | + |
| 101 | +<a name=cio> |
| 102 | + |
| 103 | +# Using crates from crates.io |
| 104 | + |
| 105 | +It is allowed to use crates from crates.io, though external |
| 106 | +dependencies should not be added gratuitously. All such crates must |
| 107 | +have a suitably permissive license. There is an automatic check which |
| 108 | +inspects the Cargo metadata to ensure this. |
| 109 | + |
| 110 | +<a name=er> |
| 111 | + |
| 112 | +# How to structure your PR |
| 113 | + |
| 114 | +How you prepare the commits in your PR can make a big difference for the reviewer. |
| 115 | +Here are some tips. |
| 116 | + |
| 117 | +**Isolate "pure refactorings" into their own commit.** For example, if |
| 118 | +you rename a method, then put that rename into its own commit, along |
| 119 | +with the renames of all the uses. |
| 120 | + |
| 121 | +**More commits is usually better.** If you are doing a large change, |
| 122 | +it's almost always better to break it up into smaller steps that can |
| 123 | +be independently understood. The one thing to be aware of is that if |
| 124 | +you introduce some code following one strategy, then change it |
| 125 | +dramatically (versus adding to it) in a later commit, that |
| 126 | +'back-and-forth' can be confusing. |
| 127 | + |
| 128 | +**If you run rustfmt and the file was not already formatted, isolate |
| 129 | +that into its own commit.** This is really the same as the previous |
| 130 | +rule, but it's worth highlighting. It's ok to rustfmt files, but since |
| 131 | +we do not currently run rustfmt all the time, that can introduce a lot |
| 132 | +of noise into your commit. Please isolate that into its own |
| 133 | +commit. This also makes rebases a lot less painful, since rustfmt |
| 134 | +tends to cause a lot of merge conflicts, and having those isolated |
| 135 | +into their own commit makes htem easier to resolve. |
| 136 | + |
| 137 | +**No merges.** We do not allow merge commits into our history, other |
| 138 | +than those by bors. If you get a merge conflict, rebase instead via a |
| 139 | +command like `git rebase -i rust-lang/master` (presuming you use the |
| 140 | +name `rust-lang` for your remote). |
| 141 | + |
| 142 | +**Individual commits do not have to build (but it's nice).** We do not |
| 143 | +require that every intermediate commit successfully builds -- we only |
| 144 | +expect to be able to bisect at a PR level. However, if you *can* make |
| 145 | +individual commits build, that is always helpful. |
| 146 | + |
0 commit comments