[pygrep_hooks] Fix blanket-noqa panic when last line has noqa with no newline (PGH004)

[augustelalande]

Summary

Resolves #11102

The error stems from these lines

ruff/crates/ruff_linter/src/noqa.rs

Lines 697 to 702 in f5c7a62

	// Extend a mapping at the end of the file to also include the EOF token.
	if let Some(last) = directives.last_mut() {
	if last.range.end() == locator.contents().text_len() {
	last.range = last.range.add_end(TextSize::from(1));
	}
	}

I don't really understand the purpose of incrementing the last index, but it makes the resulting range invalid for indexing into contents.

For now I just detect if the index is too high in blanket_noqa and adjust it if necessary.

Test Plan

Created fixture from issue example.

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

[charliermarsh]

This seems like a bug, if the .end() is greater than the contents... Have you looked into how that happens?

[charliermarsh]

I see the PR summary, but what happens if we remove it?

[augustelalande]

Ya it generates a fixture failure with W292_1.py

[augustelalande]

It seems the code mentioned in the PR summary was added by @MichaReiser so maybe he can shed some light.

[MichaReiser]

Uff, I wrote this code like a year ago. But what I did is uncomment that specific code in the noqa file and run all tests and the following test now fails

ruff/crates/ruff_linter/resources/test/fixtures/pycodestyle/W292_1.py

Lines 1 to 2 in 5849a75

	def fn() -> None:
	pass # noqa: W292

The problem is that we have fixes reported at the very end of the file, and there must be a way to suppress them.

The way the noqa lines are found is by searching by a given offset:

ruff/crates/ruff_linter/src/noqa.rs

Lines 722 to 734 in de46a36

	fn find_line_index(&self, offset: TextSize) -> Option<usize> {
	self.inner
	.binary_search_by(|directive| {
	if directive.range.end() < offset {
	std::cmp::Ordering::Less
	} else if directive.range.contains(offset) {
	std::cmp::Ordering::Equal
	} else {
	std::cmp::Ordering::Greater
	}
	})
	.ok()
	}

If we have two lines, than the previous_line.end() == next_line.start(). That's why the above methods searches the line using directive.end() < offset to avoid that a noqa comment suppresses a violation on the next lines.

I must admit, my "solution" is a hack and it would be nice if we could support end-of-file noqa comments in a better way.

[augustelalande]

@MichaReiser @charliermarsh I implemented a fix which addresses the issue at its source. Let me know if you like it better.

[MichaReiser]

Nit: I think I would rename this to includes_end to make it clear that the comparison should include the end range.

[MichaReiser]

The search then becomes

self inner.binary_search_by(|directive| {
	if directive.range.end() < offset {
		std::cmp::Ordering::Less
	} else if directive.range.start() >  offset {
		std::cmp::Ordering::Greater
	} 
	// At this point, end >= offset, start <= offset
	else if !directive.includes_end && directive.range.end() == offset  {
		std::cmp::Ordering::Less // I think or should it be greater?
	} else {
		std::cmp::Ordering::Equal
	}

[augustelalande]

Done. I don't know what that wasm test failure is about.

[charliermarsh]

I think it's spurious, I'll re-run it.