From d827cec0928032a90dd47384b002b80d797df369 Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Sun, 18 Dec 2016 21:24:30 +0100 Subject: [PATCH 1/5] Add another example to the mem::replace idiom This fixes #38 --- idioms/mem-replace.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/idioms/mem-replace.md b/idioms/mem-replace.md index 94469dd4..bfeeed38 100644 --- a/idioms/mem-replace.md +++ b/idioms/mem-replace.md @@ -36,6 +36,28 @@ fn a_to_b(e: &mut MyEnum) { } ``` +This also works with more variants: + +```Rust +use std::mem; + +enum MultiVariateEnum { + A { name: String }, + B { name: String }, + C, + D +} + +fn swizzle(e: &mut MultiVariateEnum) { + use self::MultiVariateEnum::*; + *e = match *e { + A { ref mut name } => B { name: mem::replace(name, String::new()) }, + B { ref mut name } => A { name: mem::replace(name, String::new()) }, + C => D, + D => C + } +} +``` ## Motivation From b06eacd8e025f147e5ec32e207d80e08301b4ac5 Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Thu, 22 Dec 2016 20:08:06 +0100 Subject: [PATCH 2/5] New anti-pattern: deny-warnings --- anti_patterns/deny-warnings.md | 99 ++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 anti_patterns/deny-warnings.md diff --git a/anti_patterns/deny-warnings.md b/anti_patterns/deny-warnings.md new file mode 100644 index 00000000..7d4379f0 --- /dev/null +++ b/anti_patterns/deny-warnings.md @@ -0,0 +1,99 @@ +# `#![deny(warnings)]` + +## Description + +A well-intentioned crate author wants to ensure their code builds without +warnings. So they annotate their crate root with + +## Example + +```rust +#![deny(warnings)] + +// All is well. +``` + +## Advantages + +It is short and will stop the build if anything is amiss. + +## Drawbacks + +By disallowing the compiler to build with warnings, a crate author opts out of +Rust's famed stability. Sometimes new features or old misfeatures need a change +in how things are done, thus lints are written that `warn` for a certain grace +period before being turned to `deny`. Sometimes APIs get deprecated, so their +use will emit a warning where before there was none. + +ALl this conspires to potentially break the build whenever something changes – +not only for the original developer, but also for those who wish to use their +code. + +Furthermore, crates that supply additional lints (e.g. [rust-clippy]) can no +longer be used unless the annotation is removed. + +## Alternatives + +luckily, there are two ways of tackling this problem: First, we can decouple +the build setting from the code, and second, we can name the lints we want to +deny explicitly. + +The following command line will build with all warnings set to `deny`: + +```RUSTFLAGS="-D warnings" cargo build"``` + +This can be done by any individual developer (or be set in a CI tool like +travis, but remember that this may break the build when something changes) +without requiring a change to the code. + +Alternatively, we can specify the lints that we want to `deny` in the code. +Here is a list of warning lints that is (hopefully) safe to deny: + +```rust +#[deny(bad-style, + const-err, + dead-code, + extra-requirement-in-impl, + improper-ctypes, + legacy-directory-ownership, + non-shorthand-field-patterns, + no-mangle-generic-items, + overflowing-literals, + path-statements , + patterns-in-fns-without-body, + plugin-as-library, + private-in-public, + private-no-mangle-fns, + private-no-mangle-statics, + raw-pointer-derive, + safe-extern-statics, + unconditional-recursion, + unions-with-drop-fields, + unused, + unused-allocation, + unused-comparisons, + unused-parens, + while-true)] +``` + +In addition, the following `allow`ed lints may be a good idea to `deny`: + +```rust +#[deny(missing-docs, + trivial-casts, + trivial-numeric-casts, + unused-extern-crates, + unused-import-braces, + unused-qualifications, + unused-results)] +``` + +Note that we explicitly did not set `deprecated`. + +## See also + +- Type `rustc -W help` for a list of lints on your system. Also type +`rustc --help` for a general list of options +- [rust-clippy] is a collection of lints for better Rust code + +[rust-clippy]: https://github.com/Manishearth/rust-clippy From 8c0af985a4d1453a4535a2bd2531a71e3b2058c0 Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Mon, 26 Dec 2016 09:32:18 +0100 Subject: [PATCH 3/5] added deprecated link --- anti_patterns/deny-warnings.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/anti_patterns/deny-warnings.md b/anti_patterns/deny-warnings.md index 7d4379f0..c80415f1 100644 --- a/anti_patterns/deny-warnings.md +++ b/anti_patterns/deny-warnings.md @@ -92,8 +92,10 @@ Note that we explicitly did not set `deprecated`. ## See also +- [`#[deprecated]`] documentation - Type `rustc -W help` for a list of lints on your system. Also type `rustc --help` for a general list of options - [rust-clippy] is a collection of lints for better Rust code [rust-clippy]: https://github.com/Manishearth/rust-clippy +[`#[deprecated]`]: https://doc.rust-lang.org/reference.html#miscellaneous-attributes From 81d5d4db00ff10cb9ad9c5e832b9a96f53923b43 Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Mon, 26 Dec 2016 10:02:03 +0100 Subject: [PATCH 4/5] fix link title, add explanation why not deprecated --- anti_patterns/deny-warnings.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/anti_patterns/deny-warnings.md b/anti_patterns/deny-warnings.md index c80415f1..1c84ea0f 100644 --- a/anti_patterns/deny-warnings.md +++ b/anti_patterns/deny-warnings.md @@ -88,14 +88,14 @@ In addition, the following `allow`ed lints may be a good idea to `deny`: unused-results)] ``` -Note that we explicitly did not set `deprecated`. +Note that we explicitly did not set the [deprecate attribute] ## See also -- [`#[deprecated]`] documentation +- [deprecate attribute] documentation - Type `rustc -W help` for a list of lints on your system. Also type `rustc --help` for a general list of options - [rust-clippy] is a collection of lints for better Rust code [rust-clippy]: https://github.com/Manishearth/rust-clippy -[`#[deprecated]`]: https://doc.rust-lang.org/reference.html#miscellaneous-attributes +[deprecate attribute]: https://doc.rust-lang.org/reference.html#miscellaneous-attributes From f08e132761c379bd466c19c797d0b8d132363cef Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Wed, 28 Dec 2016 00:56:58 +0100 Subject: [PATCH 5/5] incorporated hints from lfairy --- anti_patterns/deny-warnings.md | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/anti_patterns/deny-warnings.md b/anti_patterns/deny-warnings.md index 1c84ea0f..bcdaa95c 100644 --- a/anti_patterns/deny-warnings.md +++ b/anti_patterns/deny-warnings.md @@ -3,7 +3,7 @@ ## Description A well-intentioned crate author wants to ensure their code builds without -warnings. So they annotate their crate root with +warnings. So they annotate their crate root with the following: ## Example @@ -22,28 +22,33 @@ It is short and will stop the build if anything is amiss. By disallowing the compiler to build with warnings, a crate author opts out of Rust's famed stability. Sometimes new features or old misfeatures need a change in how things are done, thus lints are written that `warn` for a certain grace -period before being turned to `deny`. Sometimes APIs get deprecated, so their -use will emit a warning where before there was none. +period before being turned to `deny`. -ALl this conspires to potentially break the build whenever something changes – -not only for the original developer, but also for those who wish to use their -code. +For example, it was discovered that a type could have two `impl`s with the same +method. This was deemed a bad idea, but in order to make the transition smooth, +the `overlapping-inherent-impls` lint was introduced to give a warning to those +stumbling on this fact, before it becomes a hard error in a future release. + +Also sometimes APIs get deprecated, so their use will emit a warning where +before there was none. + +All this conspires to potentially break the build whenever something changes. Furthermore, crates that supply additional lints (e.g. [rust-clippy]) can no longer be used unless the annotation is removed. ## Alternatives -luckily, there are two ways of tackling this problem: First, we can decouple -the build setting from the code, and second, we can name the lints we want to -deny explicitly. +There are two ways of tackling this problem: First, we can decouple the build +setting from the code, and second, we can name the lints we want to deny +explicitly. The following command line will build with all warnings set to `deny`: ```RUSTFLAGS="-D warnings" cargo build"``` This can be done by any individual developer (or be set in a CI tool like -travis, but remember that this may break the build when something changes) +Travis, but remember that this may break the build when something changes) without requiring a change to the code. Alternatively, we can specify the lints that we want to `deny` in the code. @@ -79,7 +84,8 @@ Here is a list of warning lints that is (hopefully) safe to deny: In addition, the following `allow`ed lints may be a good idea to `deny`: ```rust -#[deny(missing-docs, +#[deny(missing-debug-implementations, + missing-docs, trivial-casts, trivial-numeric-casts, unused-extern-crates, @@ -88,7 +94,10 @@ In addition, the following `allow`ed lints may be a good idea to `deny`: unused-results)] ``` -Note that we explicitly did not set the [deprecate attribute] +Some may also want to add `missing-copy-implementations` to their list. + +Note that we explicitly did not add the `deprecated` lint, as it is fairly +certain that there will be more deprecated APIs in the future. ## See also