rokucommunity / bslint

A linter for BrightScript and BrighterScript.
MIT License
28 stars 15 forks source link

Avoid false positive for missing return #78

Closed xgouchet closed 1 year ago

xgouchet commented 2 years ago

Given a function with throw statements in some branches, there is a false positive that considers those branches as invalid. A throw can be a valid exit strategy for a function when "something truly terrible happens".

function ok7() as integer
    a = 1
    if a > 0
        throw "something wrong"
    else
        return 0
    end if
end function
xgouchet commented 2 years ago

This also upgrades the Brighterscript dependency to 0.59.0 to use the isThrowStatement method.

xgouchet commented 2 years ago

Seems like some tests are failing in CI with the 0.59.0 upgrade, I'll fix those asap 👍

elsassph commented 2 years ago

Thanks, the return tracking was implemented before throw was introduced!

elsassph commented 2 years ago

Can you check if it fixes #68

xgouchet commented 2 years ago

Can you check if it fixes #68

No it doesn't but I'll add that to my PR

elsassph commented 2 years ago

Code looks good, but needs testing at scale.

TwitchBronBron commented 2 years ago

I can test this on our app today.

TwitchBronBron commented 1 year ago

Sorry for the delay. I finally got around to testing this, and it looks like it works well against our larger project! The only change I would suggest is bumping the peerDependency value up since this will fail for older brighterscript versions.