Your org has enabled the Graphite merge queue for merging into main

Add the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

• fix(linter): panic in yoda #7679 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

CodSpeed Performance Report

Merging #7679 will not alter performance

_{Comparing c/12-05-fix_linter_panic_in_yoda_ (7cee065) with main (72b5d58)}

Summary

✅ 29 untouched benchmarks

Merge activity

• Dec 5, 10:33 PM EST: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
• Dec 5, 10:33 PM EST: A user added this pull request to the Graphite merge queue.
• Dec 5, 10:38 PM EST: A user merged this pull request with the Graphite merge queue.

@camc314 Sorry I'm late to the party! I think this is a bit simpler and probably more performant:

let search_start_position = expr.left.span().end;
let operator_position_start = source_str.as_bytes().windows(operator_str.len()).enumerate().find_map(|(index, chunk)| {
    if chunk == operator_str.as_bytes() {
        let pos_start = index as u32 + search_start_position;
        let pos_end = pos_start + operator_str.len() as u32;
        if !ctx.comments().iter().any(|comment| comment.span.start <= pos_start && pos_end <= comment.span.end) {
            return Some(pos_start);
        }
    }
    None
}).unwrap();

Iterating over chars is generally slower than iterating over bytes. Although we have to handle Unicode chars for correctness, in practice code rarely contains them, so bytes and chars are generally equivalent. So the extra work involved in decoding strings to chars is unnecessary.

Note 1: I've used find_map not for purpose of optimization, but because I think it makes the code simpler.

Note 2: I don't think let Some(foo) = bar else { debug_assert!(false); ... }; is any better than unwrap(). Either way, there's a branch, and you can't avoid that. In fact unwrap() may be better because it hints to compiler that the branch for None will never be taken, and it can arrange the code so the branch for Some is the fast path.

As a further optimization, you could maybe narrow down the comments which need searching to only those between expr.left.span().end and expr.right.span().start before the windows loop. Then in the loop it only has to search through this shorter selection of comments (probably 0 length), rather than searching all comments on each turn of the loop.

I was unaware of the windows iterator before this PR. TIL!