Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

more defensive coding: guard against a non-iterable loclist #4912

Merged
merged 1 commit into from
Mar 8, 2025

Conversation

ahrex
Copy link
Contributor

@ahrex ahrex commented Feb 23, 2025

Sometimes s:HandleExit can execute a deferred linter callback, which ends up setting the l:loclist that's passed into ale#engine#HandleLoclist at the end of s:HandleExit to a dictionary. This dictionary cannot be iterated over, and thus errors out.

Guard against trying to iterate over values that don't make sense.

@idbrii
Copy link
Contributor

idbrii commented Feb 26, 2025

ale#engine#FixLocList doesn't do much if it receives a dictionary loclist. If it's never valid to pass a dict to it, could we prevent the error closer to where it's triggered to make it more obvious what's gone wrong? (Presumably there's a bug in some linter callback that's causing this problem?)

Instead, maybe s:HandleExit could do an :echoerr if l:linter.callback produced a dictionary and set l:loclist = []. That would make the issue more obvious to anyone writing linters and prevent further errors. Or swallow the error at that point.

@ahrex
Copy link
Contributor Author

ahrex commented Feb 27, 2025

Updated to match this guidance:

Or swallow the error at that point.

I chose to use a try/catch with a for loop over checking the type of l:loclist because it emulates what ale#engine#FixLocList would do down the call stack, ensuring fidelity.

Thanks!

Sometimes `s:HandleExit` can execute a deferred linter callback, which
ends up setting the `l:loclist` that's passed into
`ale#engine#HandleLoclist` at the end of `s:HandleExit` to a dictionary.
This dictionary cannot be iterated over, and thus errors out.

Guard against trying to iterate over values that don't make sense.
Copy link
Contributor

@hsanson hsanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@hsanson hsanson merged commit a6db6c9 into dense-analysis:master Mar 8, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants