rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.2k stars 12.56k forks source link

Lints for target_* cfg #85753

Closed est31 closed 1 year ago

est31 commented 3 years ago

Inspired by seeing #85738 fixing a typo by replacing target_os = "opensbd" with target_os = "openbsd".

I wonder whether cases like this could be discovered by lints. There are multiple target_* values that rustc sets, and it would be interesting to think about validating them.

Rust has support for custom target json files, and ideally you would want to have older rustc versions not warn about code supporting targets added by future rustc versoins, making a warn-by-default lint that complains about any non-builtin target_* value a bit tricky.

However, one could think about an allow-by-default non_builtin_target_cfg lint, as well as a warn-by-default target_cfg_typo lint that checks for targets in a close edit distance to a builtin target. If you enable the non_builtin_target_cfg lint, maybe the target_cfg_typo could be silenced.

As prior art I was only able to find a clippy lint.

est31 commented 3 years ago

@rustbot label A-lint

est31 commented 3 years ago

Running a cursory check to find cfg typos across the rust codebase, I couldn't find any:

``` $ rg -oI 'target_[a-z]* = "[a-z]*"' | sed 's/.* = "\([a-z]*\)".*/\1/' | sort | uniq -c | sort -rn 3669 arm 298 linux 181 android 164 macos 161 emscripten 120 little 108 freebsd 104 windows 100 vxworks 81 netbsd 80 ios 77 openbsd 74 dragonfly 61 fuchsia 58 powerpc 55 solaris 53 redox 53 msvc 47 sgx 47 gnu 42 mips 42 illumos 32 haiku 28 big 23 musl 23 mclass 21 uwp 17 fortanix 14 hermit 14 atomics 13 wasi 12 none 12 asmjs 9 unix 8 x 7 sse 6 vsx 6 neon 6 hexagon 6 cloudabi 6 altivec 5 newlib 4 sparc 4 pmull 4 nvptx 4 msa 4 freertos 4 dsp 4 avx 3 uclibc 3 libnx 3 2 xsaves 2 xsaveopt 2 xsavec 2 xsave 2 watchos 2 unknown 2 tvos 2 tsc 2 tbm 2 sve 2 sha 2 rtm 2 rdseed 2 rdrand 2 rdm 2 rcpc 2 psp 2 popcnt 2 pclmulqdq 2 mmx 2 lzcnt 2 lse 2 fxsr 2 fp 2 fma 2 eabihf 2 dotprod 2 crypto 2 crc 2 apple 2 aes 2 adx 2 abm 1 spirv 1 avr ```

Maybe it's not that valuable to add this if this doesn't occur in the wild very often?

est31 commented 2 years ago

cc #82450

est31 commented 1 year ago

Yeah I think this can be closed in favour of #82450