Lint against declaring `fn to_string(&self) -> String`

[kennytm]

Emit a lint when a type defines an inherent to_string() method. One should implement Display instead.

impl S {
    fn to_string(&self) -> String {  ...  }
//  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
}

If a type both implements Display and defines an inherent to_string() method, turn up the warn level to error, since the inherent to_string() will shadow the Display impl and they may behave differently.

Do not emit the lint if to_string() is a trait method or free function.

Do not emit the lint if the method is not exactly fn(&self) -> String, or maybe still emit a lint but suggest renaming the method.

[Darth-Revan]

I think such a lint would make sense.

I'd love to give it a try implementing it as my first real contribution to this project if nobody has any objections. Looking into the various APIs this should be doable in a manageable amount of code by taking advantage of the functions defined in utils.

The only thing I'm not quite sure about is turning up the report level from warn to error. Correct me if I'm wrong, but as far as I have understood the report level of a lint is indirectly defined by its lint category (correctness, style, ...). The actual level is not defined by the lint itself, but by the macro used to register the lints (see lib.rs), where for example lints from the category correctness are mapped to Deny (emitting an error), whereas style is mapped to Warn.

So from my point of view, this would require two separate, but quite similar lints:

1. Checks for the function to_string(&self) -> String and emits a warning if a type implements it (style category).
2. Performs the same check as 1, but also checks if the same type implements Display and then emit an error (correctness category)

From an implementation perspective this wouldn't be a real problem, of course, just requires a bit more boilerplate to register the lints. Or is there some nice functionality I've missed that allows for using different report levels at runtime?

[kennytm]

@Darth-Revan Yes basically what you described. For instance both INTO_ITER_ON_ARRAY (correctness category) and INTO_ITER_ON_REF (style category) share the same check in

rust-clippy/clippy_lints/src/methods/mod.rs

Lines 2340 to 2362 in be3d6cf

	fn ty_has_iter_method(
	cx: &LateContext<'_, '_>,
	self_ref_ty: Ty<'_>,
	) -> Option<(&'static Lint, &'static str, &'static str)> {
	if let Some(ty_name) = has_iter_method(cx, self_ref_ty) {
	let lint = if ty_name == "array" || ty_name == "PathBuf" {
	INTO_ITER_ON_ARRAY
	} else {
	INTO_ITER_ON_REF
	};
	let mutbl = match self_ref_ty.sty {
	ty::Ref(_, _, mutbl) => mutbl,
	_ => unreachable!(),
	};
	let method_name = match mutbl {
	hir::MutImmutable => "iter",
	hir::MutMutable => "iter_mut",
	};
	Some((lint, ty_name, method_name))
	} else {
	None
	}
	}

[Darth-Revan]

@kennytm Thank you very much. That's exactly what I was looking for. I'll have a look into that and try to adapt it for the new lint.