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

Add capability to "filter" certain functions out of the warning linter that don't return a value that needs to be processed #682

Open
creikey opened this issue Apr 8, 2020 · 5 comments

Comments

@creikey
Copy link

creikey commented Apr 8, 2020

Describe the project you are working on:
A medium to large sized action adventure shmup.
Describe the problem or limitation you are having in your project:
Too many warnings about not processing a return value from functions like connect or change_scene

Because some functions should always return a normal value on normal operation, and whenever they return a faulty error, the debugger tells me, under no circumstances should I need to process the error result programmatically.
Describe the feature / enhancement and how it helps to overcome the problem or limitation:
A checkbox in the Gdscript subsection of the Debug section in Project Settings that will exclude functions like connect where it's unlikely you'll ever need to process the return value in games, excluding those functions from the warning section.
Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
image

If this enhancement will not be used often, can it be worked around with a few lines of script?:
Yes, you could add an ignore warning comment for every single function like connect that shouldn't return an erroneous value that must be processed programmatically, however, this is prone to error when you have a lot of warnings built up in a project ( you may ignore the actual errors ) and slows down development time.
Is there a reason why this should be core and not an add-on in the asset library?:
Decisions on what functions shouldn't ever return a value that needs to be processed need to be made carefully so as to avoid each project reporting different errors, confusing developers.

@creikey creikey changed the title Add capability to "filter" certain functions out of the warning linter Add capability to "filter" certain functions out of the warning linter that don't return a value that needs to be processed Apr 8, 2020
@creikey
Copy link
Author

creikey commented Apr 8, 2020

A capability like this was suggested by @akien-mga in godotengine/godot#27576 :

Possible fixes:

    Make connect return void (breaks compat)
    Add a whitelist of builtin methods that should not trigger this warning

@CharlesJenkins
Copy link

I suggest adopting C#'s "_ =" syntax for indicating a return value ignored on purpose:

_ = connect( "pressed", self, "_on_Button_pressed" )

It's not nearly as much visual clutter as the comment that disables the warning, it clearly communicates the programmer's intent, and it keeps us from being tempted to disable a warning that could be valuable for solving difficult bugs.

@Calinou
Copy link
Member

Calinou commented Nov 13, 2022

Related to #613.

Personally, I wouldn't make only certain functions have their warning ignored (especially if such a list is used by default). This adds too many opportunities for subtle programming bugs to occur. There are no hidden bugs until there are – you can't predict people's use cases for all engine functions 🙂

Instead, I suggest adding a @discard annotation (similar to Nim's discard keyword) to be used as follows:

@discard some_function()

Allowing var _ = ... to be used as a special variable (can't read its value, but can assign it as many times as desired) could work too, but it feels like you're "exploiting the language" so to speak.

@m-r-hunt
Copy link

m-r-hunt commented Nov 13, 2022

A good example of return values that will be very frequently ignored with no issue are the tween functions in Tweener - for example tween_method returns a MethodTweener object which you only need if you're customising the tween further than the original call.

Similarly methods that are designed to be "chained" by returning the same object again may well run into this warning.

Having either no warning in cases like these or a very concise way to get rid of the warning would be good in my opinion.

@YuriSizov
Copy link
Contributor

YuriSizov commented Nov 14, 2022

We've discussed this in the contributors chat a few days ago and seem to have agreed that it makes sense to only raise this warning for const methods, as these methods by definition don't modify anything, and in most, if not all, cases exist solely to return a value (so ignoring it is a certain error).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants