Ruff: Add and fix RUF015

[kiblik]

Originally proposed in #10712 but I didn't like Ruff's autofix

This PR is trying to address unnecessary-iterable-allocation-for-first-element (RUF015)

next(iter(x)) is proposed by Ruff but it is a bit hard to read it in code so I wrote a wrapper. Plus, I dropped som conversions to a list where it is not needed.

[dryrunsecurity[bot]]

DryRun Security Summary

A code review revealed multiple files were patched to improve code readability, with identified security concerns including minimal JSON data validation, potential information exposure, access control considerations, and a hardcoded API endpoint in specific files.

Expand for full summary

Summary: Multiple files were patched to replace list indexing with a new utility function first_elem() from dojo.utils, primarily focusing on code refactoring and readability improvements.

Security Findings:

• Input Validation Concern in dojo/reports/widgets.py:

  • Minimal JSON data validation
  • Direct json.loads() could be vulnerable to JSON parsing issues

• Potential Information Exposure in dojo/reports/widgets.py:

  • Risk of inadvertently exposing sensitive information during report generation

• Access Control Considerations in dojo/reports/widgets.py:

  • Requires careful validation of user permissions before report generation

• Hardcoded API Endpoint in dojo/tools/api_cobalt/api_client.py:

  • Uses hardcoded API endpoint (https://api.cobalt.io)

No other direct security vulnerabilities were identified across the reviewed files.

View PR in the DryRun Dashboard.

[Maffooch]

Hey @kiblik I am super conflicted about this one, but have not landed on a stance I feel great about.

On one hand, I like the approach of abstracting away next(iter(x)) as it does not appear all that intuitive at first glance. On the other hand, this takes away the opportunity to learn why the use of the iterator is the safer and performant approach. I don't have much experience in iterators in python, and learned something very valuable from just this PR 😄

I have no qualms with the ruff rule introduced here, but am on the fence about the use of the utility (we have a ton of those as it is 😅 )

[kiblik]

Hey @kiblik I am super conflicted about this one, but have not landed on a stance I feel great about.

On one hand, I like the approach of abstracting away next(iter(x)) as it does not appear all that intuitive at first glance. On the other hand, this takes away the opportunity to learn why the use of the iterator is the safer and performant approach. I don't have much experience in iterators in python, and learned something very valuable from just this PR 😄

I have no qualms with the ruff rule introduced here, but am on the fence about the use of the utility (we have a ton of those as it is 😅 )

I had also a bit of mixed feelings about this one as well. Regarding iterators, they might help us with time and space efficiency. I can imagine that we can use it in the dojo/ but not in unittests/ (where that efficiency is not needed that much compared to running up). I would like to know also other's opinions about this.

[dogboat]

The main question is whether to introduce another util method and use it everywhere, right? We want to switch to using next(iter(x)) either directly or indirectly (via util method) either way?

[Maffooch]

Yep, to util, or not util 😂

[dogboat]

Gotcha, thanks. Yeah, tough one! I'll make some arguments for and against.

For: I think first_elem reads nicer and imho is more obvious as to what it does (I agree with the comment: "next(iter(x)) is harder for reading"). next(iter(x)) does raise an exception on an empty or None x; I don't think we need to worry about this in any of the current cases (we would've already seen index exceptions being raised), but using a util method would allow us to easily accept/supply a default to next() or handle Nones so we don't hit such exceptions.

Against: next(iter(x)) is already the Pythonic way to do this and (...probably) won't need to be rewritten anytime soon, so it'd be fine to just have it embedded throughout the codebase. It also serves, as Cody pointed out, as a good teaching tool. Adding a util method that's only a single line doesn't buy us much except for the much more clear name. While clarity is important, in this case I'd argue that someone only needs to learn what the "more cryptic" version does once. And I'll be honest, I'll probably be the first to forget the util method exists and would have to be reminded. 😞

We could also split the difference: reports/widgets.py is the only place where this is used many times, so we could define and use the method there, and use next(iter(x)) everywhere else. That way everybody's unhappy. 😉

FWIW I lean slightly toward not using the util method (at least as-is), but I won't shout into my pillow tonight if we decide to go with it.

[Maffooch]

Let's do the next(iter(x)) in all the places for now. If we need to pivot int he future, we can

[github-actions[bot]]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions[bot]]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[github-actions[bot]]

This pull request has conflicts, please resolve those before we can evaluate the pull request.