Ignore react list keys

[psugihara-od]

Is your feature request related to a problem? Please describe.

We run talisman on every modified file in a pre-commit hook. In general, this is great and catches possible violations. However, it always incorrectly flags list keys in .jsx and .tsx files as low severity offenses. This results in engineers adding the --no-verify flag when they're committing work and sometimes missing other important errors from the pre-commit hook.

Describe the solution you'd like It would be awesome if talisman ignored the key prop in jsx/tsx components.

Describe alternatives you've considered We could add this to a local ignore file for our repos, but that wouldn't benefit other users of talisman in our org and the broader community.

Additional context This is an awesome tool, thanks for maintaining it!

Here's an example of an instance of key that got flagged (please see the linked docs for more info):

{items.map(({ title, current, done, body }) => (
  <div
    key={itemsKey + title}
    className={b("item", { current, done })()}
  >
    <div className={b("item-inner")()}>
      <div className={b("item-title").mix("ff-bold")()}>
        {title}
      </div>
      <div className={b("item-body")()}>{body}</div>
    </div>
  </div>
))}

[harinee]

Thanks @psugihara-od I hear you. The concern I have is where one could have an entry of an actual key/secret value getting checked in, since jsx/tsx is a piece of code that is still in control of the devs. If we were to ignore jsx or tsx files, we could go by the path of Ignoring by language scope (will need little code to be written in Talisman), but risk that all other secrets in these files go ignored as well. Thoughts?

By the way, one other way of dealing with this is to set your threshold to medium

[svishwanath-tw]

@psugihara-od : You could try <[a-zA-Z0-9-_]+\s+key=(\{[^}]+\}|'[^']+'|"[^"]+") as an allowed pattern like below in your .talismanrc

allowed_patterns:
- <[a-zA-Z0-9-_]+\s+key=(\{[^}]+\}|'[^']+'|"[^"]+")

I've tested it out here: https://regex101.com/r/g9h2R8/1

[psugihara-od]

Thanks both @harinee and @svishwanath-tw for workarounds! We turned these into warnings rather than errors and now it's less safe but just a minor and workable annoyance.

Closing this for now (re-open if useful), but I'd vote for this to be handled automatically by Talisman as a feature enhancement down the road since this is idiomatic usage of a very popular web framework. I think in an ideal world Talisman would take on the burden of eliminating common false positives from its detector rather than have them in user configs.

At a first glance, it seems like one way to enable this type of detection is to have some specific patterns by file type rather than running all of these on all files.

[mauriciomelo]

I would love to have this built into Talisman. On React codebases, false positives become the norm since so many JSX files contain a key prop. I'm afraid it makes Talisman hard to justify on such codebases when we have to ignore so many files. I noticed that, when it's too noisy, people start to blindly ignore errors. This could potentially hide real issues.

Thanks for the workaround!

Edit: I ended up setting the threshold to medium and it solved most of the issues