summaryrefslogtreecommitdiffstats
path: root/src/tools/clippy/clippy_lints/src/functions/mod.rs
blob: d2852b4acad1eca80da81340410c0953c09ff651 (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
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
mod impl_trait_in_params;
mod misnamed_getters;
mod must_use;
mod not_unsafe_ptr_arg_deref;
mod result;
mod too_many_arguments;
mod too_many_lines;

use rustc_hir as hir;
use rustc_hir::intravisit;
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::def_id::LocalDefId;
use rustc_span::Span;

declare_clippy_lint! {
    /// ### What it does
    /// Checks for functions with too many parameters.
    ///
    /// ### Why is this bad?
    /// Functions with lots of parameters are considered bad
    /// style and reduce readability (“what does the 5th parameter mean?”). Consider
    /// grouping some parameters into a new type.
    ///
    /// ### Example
    /// ```rust
    /// # struct Color;
    /// fn foo(x: u32, y: u32, name: &str, c: Color, w: f32, h: f32, a: f32, b: f32) {
    ///     // ..
    /// }
    /// ```
    #[clippy::version = "pre 1.29.0"]
    pub TOO_MANY_ARGUMENTS,
    complexity,
    "functions with too many arguments"
}

declare_clippy_lint! {
    /// ### What it does
    /// Checks for functions with a large amount of lines.
    ///
    /// ### Why is this bad?
    /// Functions with a lot of lines are harder to understand
    /// due to having to look at a larger amount of code to understand what the
    /// function is doing. Consider splitting the body of the function into
    /// multiple functions.
    ///
    /// ### Example
    /// ```rust
    /// fn im_too_long() {
    ///     println!("");
    ///     // ... 100 more LoC
    ///     println!("");
    /// }
    /// ```
    #[clippy::version = "1.34.0"]
    pub TOO_MANY_LINES,
    pedantic,
    "functions with too many lines"
}

declare_clippy_lint! {
    /// ### What it does
    /// Checks for public functions that dereference raw pointer
    /// arguments but are not marked `unsafe`.
    ///
    /// ### Why is this bad?
    /// The function should almost definitely be marked `unsafe`, since for an
    /// arbitrary raw pointer, there is no way of telling for sure if it is valid.
    ///
    /// In general, this lint should **never be disabled** unless it is definitely a
    /// false positive (please submit an issue if so) since it breaks Rust's
    /// soundness guarantees, directly exposing API users to potentially dangerous
    /// program behavior. This is also true for internal APIs, as it is easy to leak
    /// unsoundness.
    ///
    /// ### Context
    /// In Rust, an `unsafe {...}` block is used to indicate that the code in that
    /// section has been verified in some way that the compiler can not. For a
    /// function that accepts a raw pointer then accesses the pointer's data, this is
    /// generally impossible as the incoming pointer could point anywhere, valid or
    /// not. So, the signature should be marked `unsafe fn`: this indicates that the
    /// function's caller must provide some verification that the arguments it sends
    /// are valid (and then call the function within an `unsafe` block).
    ///
    /// ### Known problems
    /// * It does not check functions recursively so if the pointer is passed to a
    /// private non-`unsafe` function which does the dereferencing, the lint won't
    /// trigger (false negative).
    /// * It only checks for arguments whose type are raw pointers, not raw pointers
    /// got from an argument in some other way (`fn foo(bar: &[*const u8])` or
    /// `some_argument.get_raw_ptr()`) (false negative).
    ///
    /// ### Example
    /// ```rust,ignore
    /// pub fn foo(x: *const u8) {
    ///     println!("{}", unsafe { *x });
    /// }
    ///
    /// // this call "looks" safe but will segfault or worse!
    /// // foo(invalid_ptr);
    /// ```
    ///
    /// Use instead:
    /// ```rust,ignore
    /// pub unsafe fn foo(x: *const u8) {
    ///     println!("{}", unsafe { *x });
    /// }
    ///
    /// // this would cause a compiler error for calling without `unsafe`
    /// // foo(invalid_ptr);
    ///
    /// // sound call if the caller knows the pointer is valid
    /// unsafe { foo(valid_ptr); }
    /// ```
    #[clippy::version = "pre 1.29.0"]
    pub NOT_UNSAFE_PTR_ARG_DEREF,
    correctness,
    "public functions dereferencing raw pointer arguments but not marked `unsafe`"
}

declare_clippy_lint! {
    /// ### What it does
    /// Checks for a `#[must_use]` attribute on
    /// unit-returning functions and methods.
    ///
    /// ### Why is this bad?
    /// Unit values are useless. The attribute is likely
    /// a remnant of a refactoring that removed the return type.
    ///
    /// ### Examples
    /// ```rust
    /// #[must_use]
    /// fn useless() { }
    /// ```
    #[clippy::version = "1.40.0"]
    pub MUST_USE_UNIT,
    style,
    "`#[must_use]` attribute on a unit-returning function / method"
}

declare_clippy_lint! {
    /// ### What it does
    /// Checks for a `#[must_use]` attribute without
    /// further information on functions and methods that return a type already
    /// marked as `#[must_use]`.
    ///
    /// ### Why is this bad?
    /// The attribute isn't needed. Not using the result
    /// will already be reported. Alternatively, one can add some text to the
    /// attribute to improve the lint message.
    ///
    /// ### Examples
    /// ```rust
    /// #[must_use]
    /// fn double_must_use() -> Result<(), ()> {
    ///     unimplemented!();
    /// }
    /// ```
    #[clippy::version = "1.40.0"]
    pub DOUBLE_MUST_USE,
    style,
    "`#[must_use]` attribute on a `#[must_use]`-returning function / method"
}

declare_clippy_lint! {
    /// ### What it does
    /// Checks for public functions that have no
    /// `#[must_use]` attribute, but return something not already marked
    /// must-use, have no mutable arg and mutate no statics.
    ///
    /// ### Why is this bad?
    /// Not bad at all, this lint just shows places where
    /// you could add the attribute.
    ///
    /// ### Known problems
    /// The lint only checks the arguments for mutable
    /// types without looking if they are actually changed. On the other hand,
    /// it also ignores a broad range of potentially interesting side effects,
    /// because we cannot decide whether the programmer intends the function to
    /// be called for the side effect or the result. Expect many false
    /// positives. At least we don't lint if the result type is unit or already
    /// `#[must_use]`.
    ///
    /// ### Examples
    /// ```rust
    /// // this could be annotated with `#[must_use]`.
    /// fn id<T>(t: T) -> T { t }
    /// ```
    #[clippy::version = "1.40.0"]
    pub MUST_USE_CANDIDATE,
    pedantic,
    "function or method that could take a `#[must_use]` attribute"
}

declare_clippy_lint! {
    /// ### What it does
    /// Checks for public functions that return a `Result`
    /// with an `Err` type of `()`. It suggests using a custom type that
    /// implements `std::error::Error`.
    ///
    /// ### Why is this bad?
    /// Unit does not implement `Error` and carries no
    /// further information about what went wrong.
    ///
    /// ### Known problems
    /// Of course, this lint assumes that `Result` is used
    /// for a fallible operation (which is after all the intended use). However
    /// code may opt to (mis)use it as a basic two-variant-enum. In that case,
    /// the suggestion is misguided, and the code should use a custom enum
    /// instead.
    ///
    /// ### Examples
    /// ```rust
    /// pub fn read_u8() -> Result<u8, ()> { Err(()) }
    /// ```
    /// should become
    /// ```rust,should_panic
    /// use std::fmt;
    ///
    /// #[derive(Debug)]
    /// pub struct EndOfStream;
    ///
    /// impl fmt::Display for EndOfStream {
    ///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
    ///         write!(f, "End of Stream")
    ///     }
    /// }
    ///
    /// impl std::error::Error for EndOfStream { }
    ///
    /// pub fn read_u8() -> Result<u8, EndOfStream> { Err(EndOfStream) }
    ///# fn main() {
    ///#     read_u8().unwrap();
    ///# }
    /// ```
    ///
    /// Note that there are crates that simplify creating the error type, e.g.
    /// [`thiserror`](https://docs.rs/thiserror).
    #[clippy::version = "1.49.0"]
    pub RESULT_UNIT_ERR,
    style,
    "public function returning `Result` with an `Err` type of `()`"
}

declare_clippy_lint! {
    /// ### What it does
    /// Checks for functions that return `Result` with an unusually large
    /// `Err`-variant.
    ///
    /// ### Why is this bad?
    /// A `Result` is at least as large as the `Err`-variant. While we
    /// expect that variant to be seldomly used, the compiler needs to reserve
    /// and move that much memory every single time.
    ///
    /// ### Known problems
    /// The size determined by Clippy is platform-dependent.
    ///
    /// ### Examples
    /// ```rust
    /// pub enum ParseError {
    ///     UnparsedBytes([u8; 512]),
    ///     UnexpectedEof,
    /// }
    ///
    /// // The `Result` has at least 512 bytes, even in the `Ok`-case
    /// pub fn parse() -> Result<(), ParseError> {
    ///     Ok(())
    /// }
    /// ```
    /// should be
    /// ```
    /// pub enum ParseError {
    ///     UnparsedBytes(Box<[u8; 512]>),
    ///     UnexpectedEof,
    /// }
    ///
    /// // The `Result` is slightly larger than a pointer
    /// pub fn parse() -> Result<(), ParseError> {
    ///     Ok(())
    /// }
    /// ```
    #[clippy::version = "1.65.0"]
    pub RESULT_LARGE_ERR,
    perf,
    "function returning `Result` with large `Err` type"
}

declare_clippy_lint! {
    /// ### What it does
    /// Checks for getter methods that return a field that doesn't correspond
    /// to the name of the method, when there is a field's whose name matches that of the method.
    ///
    /// ### Why is this bad?
    /// It is most likely that such a  method is a bug caused by a typo or by copy-pasting.
    ///
    /// ### Example

    /// ```rust
    /// struct A {
    ///     a: String,
    ///     b: String,
    /// }
    ///
    /// impl A {
    ///     fn a(&self) -> &str{
    ///         &self.b
    ///     }
    /// }

    /// ```
    /// Use instead:
    /// ```rust
    /// struct A {
    ///     a: String,
    ///     b: String,
    /// }
    ///
    /// impl A {
    ///     fn a(&self) -> &str{
    ///         &self.a
    ///     }
    /// }
    /// ```
    #[clippy::version = "1.67.0"]
    pub MISNAMED_GETTERS,
    suspicious,
    "getter method returning the wrong field"
}

declare_clippy_lint! {
    /// ### What it does
    /// Lints when `impl Trait` is being used in a function's paremeters.
    /// ### Why is this bad?
    /// Turbofish syntax (`::<>`) cannot be used when `impl Trait` is being used, making `impl Trait` less powerful. Readability may also be a factor.
    ///
    /// ### Example
    /// ```rust
    /// trait MyTrait {}
    /// fn foo(a: impl MyTrait) {
    /// 	// [...]
    /// }
    /// ```
    /// Use instead:
    /// ```rust
    /// trait MyTrait {}
    /// fn foo<T: MyTrait>(a: T) {
    /// 	// [...]
    /// }
    /// ```
    #[clippy::version = "1.68.0"]
    pub IMPL_TRAIT_IN_PARAMS,
    restriction,
    "`impl Trait` is used in the function's parameters"
}

#[derive(Copy, Clone)]
pub struct Functions {
    too_many_arguments_threshold: u64,
    too_many_lines_threshold: u64,
    large_error_threshold: u64,
}

impl Functions {
    pub fn new(too_many_arguments_threshold: u64, too_many_lines_threshold: u64, large_error_threshold: u64) -> Self {
        Self {
            too_many_arguments_threshold,
            too_many_lines_threshold,
            large_error_threshold,
        }
    }
}

impl_lint_pass!(Functions => [
    TOO_MANY_ARGUMENTS,
    TOO_MANY_LINES,
    NOT_UNSAFE_PTR_ARG_DEREF,
    MUST_USE_UNIT,
    DOUBLE_MUST_USE,
    MUST_USE_CANDIDATE,
    RESULT_UNIT_ERR,
    RESULT_LARGE_ERR,
    MISNAMED_GETTERS,
    IMPL_TRAIT_IN_PARAMS,
]);

impl<'tcx> LateLintPass<'tcx> for Functions {
    fn check_fn(
        &mut self,
        cx: &LateContext<'tcx>,
        kind: intravisit::FnKind<'tcx>,
        decl: &'tcx hir::FnDecl<'_>,
        body: &'tcx hir::Body<'_>,
        span: Span,
        def_id: LocalDefId,
    ) {
        let hir_id = cx.tcx.hir().local_def_id_to_hir_id(def_id);
        too_many_arguments::check_fn(cx, kind, decl, span, hir_id, self.too_many_arguments_threshold);
        too_many_lines::check_fn(cx, kind, span, body, self.too_many_lines_threshold);
        not_unsafe_ptr_arg_deref::check_fn(cx, kind, decl, body, def_id);
        misnamed_getters::check_fn(cx, kind, decl, body, span);
        impl_trait_in_params::check_fn(cx, &kind, body, hir_id);
    }

    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
        must_use::check_item(cx, item);
        result::check_item(cx, item, self.large_error_threshold);
    }

    fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::ImplItem<'_>) {
        must_use::check_impl_item(cx, item);
        result::check_impl_item(cx, item, self.large_error_threshold);
    }

    fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'_>) {
        too_many_arguments::check_trait_item(cx, item, self.too_many_arguments_threshold);
        not_unsafe_ptr_arg_deref::check_trait_item(cx, item);
        must_use::check_trait_item(cx, item);
        result::check_trait_item(cx, item, self.large_error_threshold);
    }
}