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

@warning_ignore() annotation don't work with all warnings #56592

Closed
Wildos opened this issue Jan 7, 2022 · 21 comments · Fixed by #83037
Closed

@warning_ignore() annotation don't work with all warnings #56592

Wildos opened this issue Jan 7, 2022 · 21 comments · Fixed by #83037

Comments

@Wildos
Copy link

Wildos commented Jan 7, 2022

Godot version

4.0.dev.custom_build.096a13b3b

System information

Linux, 5.16.0-1-MANJARO

Issue description

The annotation "@warning_ignore" don't ignore all warnings properly, some are still raised even with the annotation

Properly ignored warnings:

  • UNASSIGNED_VARIABLE
  • UNUSED_VARIABLE
  • UNUSED_LOCAL_CONSTANT
  • SHADOWED_VARIABLE
  • VOID_ASSIGNMENT
  • NARROWING_CONVERSION
  • INCOMPATIBLE_TERNARY
  • INTEGER_DIVISION
  • ASSERT_ALWAYS_TRUE
  • ASSERT_ALWAYS_FALSE
  • REDUNDANT_AWAIT

Not ignored warnings:

  • UNUSED_PARAMETER
  • UNASSIGNED_VARIABLE_OP_ASSIGN
  • STANDALONE_EXPRESSION
  • UNREACHABLE_CODE

Untested warnings:

  • SHADOWED_VARIABLE_BASE_CLASS (not sure what it is exactly)
  • UNUSED_PRIVATE_CLASS_VARIABLE (don't know how to make private class variable)
  • UNREACHABLE_PATTERN (don't know what it is)
  • STANDALONE_TERNARY (was raised as STANDALONE_EXPRESSION)
  • UNUSED_SIGNAL (was not raised)
  • RETURN_VALUE_DISCARDED (was not raised)

Steps to reproduce

Use this code as a new script and look at the warning section.

extends Node

@warning_ignore(unused_signal)
signal unused_signal # UNUSED_SIGNAL not shown
signal awaited_signal

class test_entity:
	var shadowed_variable
	var shadowed_variable_base_class
	var property_used_as_function
	var active
	
	func void_assign() -> void:
		return
	
	func get_two() -> int:
		return 2
	@warning_ignore(unused_parameter)
	func test_function(unused_parameter: int) -> void: # UNUSED_PARAMETER
		@warning_ignore(unassigned_variable)
		var unassigned_var # UNASSIGNED_VARIABLE
		var a = unassigned_var
		print(a)
		
		var unassigned_var_op_assign 
		@warning_ignore(unassigned_variable_op_assign)
		unassigned_var_op_assign += 1 # UNASSIGNED_VARIABLE_OP_ASSIGN
		print(unassigned_var_op_assign)
		
		@warning_ignore(unused_variable)
		var unused_variable = 1 # UNUSED_VARIABLE
		
		@warning_ignore(unused_local_constant)
		const unused_constant = 1 # UNUSED_LOCAL_CONSTANT
		@warning_ignore(shadowed_variable)
		var shadowed_variable = 2 # SHADOWED_VARIABLE
		print(shadowed_variable)
		
		@warning_ignore(standalone_expression)
		2 + 2 # STANDALONE_EXPRESSION
		
		@warning_ignore(void_assignment)
		var void_assignment = void_assign() # VOID_ASSIGNMENT
		print(void_assignment)
		
		var test_float : float = 3.3
		@warning_ignore(narrowing_conversion)
		var test_int : int = test_float # NARROWING_CONVERSION
		print("%f %d" % [test_float, test_int])
		
		@warning_ignore(incompatible_ternary)
		test_float = 1 if 1 > 2 else null #INCOMPATIBLE_TERNARY
		
		@warning_ignore(return_value_discarded)
		get_two() #RETURN_VALUE_DISCARDED not shown
		
		@warning_ignore(integer_division)
		test_int = test_int / test_int # INTEGER_DIVISION
		
		@warning_ignore(standalone_ternary)
		42 if active else 0 # raise STANDALONE_EXPRESSION instead of STANDALONE_TERNARY
		
		@warning_ignore(assert_always_true)
		assert(true) # ASSERT_ALWAYS_TRUE
		@warning_ignore(assert_always_false)
		assert(false) # ASSERT_ALWAYS_FALSE
		
		@warning_ignore(redundant_await)
		await null # REDUNDANT_AWAIT
		
		return
		@warning_ignore(unreachable_code)
		print("unreachable code") # UNREACHABLE_CODE
	
	
class entity_child extends test_entity:
	func bidule() -> void:
		@warning_ignore(shadowed_variable_base_class)
		var shadowed_variable_base_class = 2# SHADOWED_VARIABLE, should have been SHADOWED_VARIABLE_BASE_CLASS ?
		print(shadowed_variable_base_class)

Minimal reproduction project

test_warning_ignore.zip

@akien-mga akien-mga added this to the 4.0 milestone Jan 7, 2022
@Calinou
Copy link
Member

Calinou commented Jan 7, 2022

PS: This is a good opportunity to add some GDScript integration tests while fixing this issue.

@Calinou Calinou changed the title Warning_ignore annotation don't work with all warnings - Gdscript 2.0 GDScript 2.0: @warning_ignore() annotation don't work with all warnings Mar 18, 2022
@EvelynTSMG
Copy link

Can confirm UNREACHABLE_PATTERN is also not ignored:

class test_entity:
	func test_function() -> void:
		var x = 2
		
		@warning_ignore(unreachable_pattern)
		match x:
			1:
				return
			_:
				return
			3: # UNREACHABLE_PATTERN
				return

@James103
Copy link

Can confirm in 4.0 alpha 13 for the following warnings:

  • UNUSED_PARAMETER
  • UNASSIGNED_VARIABLE_OP_ASSIGN
  • STANDALONE_EXPRESSION
  • 42 if active else 0 gives STANDALONE_EXPRESSION and not STANDALONE_TERNARY
  • UNREACHABLE_CODE
  • SHADOWED_VARIABLE instead of SHADOWED_VARIABLE_BASE_CLASS
    • @warning_ignore(shadowed_variable) hides that warning.

In addition, using the code posted by @EvelynTSMG in the above comment prints one more warning which is not ignored: UNREACHABLE_PATTERN. Note that clicking the "Ignore" button to hide that warning generates multiple GDScript errors, listed as follows:

Line 11:Expected expression for match pattern.
Line 11:No pattern found for "match" branch.
Line 11:Expected ":" after "match" patterns.
Line 11:Expected ":" after "match" patterns.

@Morphinometr
Copy link

Warning for integer division not ignored if it in for loop condition

extends Node

var a : int = 6
var b : int = 2
var c : int

func _ready():
	@warning_ignore(integer_division)
	for i in range(a / b): # Warning!
		pass
	
	@warning_ignore(integer_division)
	c = a / b # No warnings

@mieldepoche
Copy link
Contributor

In 4.0 alpha 15,

@warning_ignore(shadowed_variable_base_class) and @warning_ignore(shadowed_variable) do not hide the warnings when the shadowing variable is a for loop iterator.

extends Node

var foo

func test():
    # warning ignored
    @warning_ignore(shadowed_variable_base_class)
    var name

    # warning *not* ignored
    @warning_ignore(shadowed_variable_base_class)
    for name in []:
        pass

    # warning ignored
    @warning_ignore(shadowed_variable)
    var foo

    # warning *not* ignored
    @warning_ignore(shadowed_variable)
    for foo in []:
        pass

image
image

@lapspider45
Copy link

lapspider45 commented Sep 3, 2022

[4.0 alpha 15]

Also doesn't seem to ignore INT_ASSIGNED_TO_ENUM.

extends Area2D

@warning_ignore(int_assigned_to_enum)
@export var active_process_mode:ProcessMode = PROCESS_MODE_INHERIT

This kinda smells of a different bug, since it gives the error message Integer used when an enum value is expected. If this is intended cast the integer to the enum type. and I don't think there is anything you can do to make this more explicit than it already is. Regardless, @warning_ignore does nothing here.
edit: apparently you make it explicit by writing ProcessMode.PROCESS_MODE_INHERIT

@Kubulambula
Copy link
Contributor

[4.0 beta 2]

@warning_ignore does not also work for:
UNSAFE_PROPERTY_ACCESS
UNSAFE_METHOD_ACCESS
UNSAFE_CAST
(I could not get to the UNSAFE_CALL_ARGUMENT warning)

These warnings are disabled by default so I'm reporting them so they don't slip trough somehow

@MikeSchulze
Copy link

The current documentation is very incomplete.

What is the replacement for #warning-ignore-all:unused_argument

I would guess @warning_ignore(unused_parameter) at class level
and for function on function level?

e.i

extends Resource
#warning-ignore-all:unused_parameter
#warning-ignore-all:return_value_discarded
@warning_ignore(unused_parameter)

func foo():
	pass


@warning_ignore(unused_parameter)
func bar(value :int) -> void:
	pass

But it sill reports warning when loading via load(<script_path>)
v4.0.beta2.official [f8745f2]

W 0:00:00:0782   The parameter 'value' is never used in the function 'bar'. If this is intended, prefix it with an underscore: '_value'
  <C++-Fehler>   UNUSED_PARAMETER
  <Quelle>       new_script.gd:11

@MikeSchulze
Copy link

Hi on Godot 3 we was able to ignore ALL warnings at class level
#warning-ignore-all:unused_argument

On Godot 4 i missed this option
@warning_ignore_all('unused_parameter')

Please bring it back.
I use test suites with test functions that have optional arguments to control test execution.

func test_foo(timeout := 2000) -> void:
   ....

This parameter is only used by the internal test executor to terminate the test after the timeout.
So I don't want to force the test author to add the following annotations to every test where these parameters are used.
@warning_ignore('unused_parameter')

To avoid this, I would like it to return to the ability to disable all warnings at the class level.
@warning_ignore_all(<warning_type>)

And @warning_ignore('unused_parameter') is broken on v4.0.beta16.official [518b9e5]

@gelvinp
Copy link
Contributor

gelvinp commented Jan 28, 2023

@MikeSchulze

Adding a @warning_ignore(warning-names-here) at the top of the file, below the extends line will ignore those warnings for the whole file, with the caveat that some of the ignores, like unused parameter, currently do not work.

@MikeSchulze
Copy link

@MikeSchulze

Adding a @warning_ignore(warning-names-here) at the top of the file, below the extends line will ignore those warnings for the whole file, with the caveat that some of the ignores, like unused parameter, currently do not work.

Ah nice, so it should work :)
Then we still have to wait for a fix for unused_parameter

@snoopdouglas
Copy link
Contributor

INTEGER_DIVISION doesn't seem to be able to be ignored as of beta 17.

@vnen vnen modified the milestones: 4.0, 4.1 Feb 24, 2023
@MikeSchulze
Copy link

Godot v4.0.stable.official [92bee43]

ignore shadowed_global_identifier is not working

@warning_ignore("unused_parameter", "shadowed_global_identifier")
func is_equal_approx(expected :Vector2, approx :Vector2) -> GdUnitVector2Assert:

using @warning_ignore("unused_parameter") after extends does not work for the whole class

MikeSchulze added a commit to MikeSchulze/gdUnit4 that referenced this issue Mar 9, 2023
# Why
By default addons are excluded from code validation, after inclusion the
plugin displays over 1800 warnings and some errors

![image](https://user-images.githubusercontent.com/347037/223846970-74dc362b-dcd2-48a0-9a6b-7137e4c1671b.png)


# What
- fix the errors
- reduce the warnings
- update inline documentation  
- apply formatting rules

# Open issues
-Only 23 warnings left, covered by the listed Godot issues

![image](https://user-images.githubusercontent.com/347037/224134816-2f3729bd-4aac-4cc5-af6d-3934b49a1b2f.png)

- invalid warnings about enum
[74593](godotengine/godot#74593)
  - INT_AS_ENUM_WITHOUT_CAST
  - INT_AS_ENUM_WITHOUT_MATCH 
- disable warnings not work
[56592](godotengine/godot#56592)
   - shadowed_global_identifier
@MikeSchulze
Copy link

Is there an update here? Would be nice if the ignore annotations would also work as described ;)

@dalexeev
Copy link
Member

dalexeev commented May 3, 2023

Is there an update here?

There are no updates yet, as far as I know. I plan to take a look later (added to my TODO list).

@MikeSchulze
Copy link

Is there an update here?

There are no updates yet, as far as I know. I plan to take a look later (added to my TODO list).

Thank you, that sounds very good. 👍

@akien-mga akien-mga changed the title GDScript 2.0: @warning_ignore() annotation don't work with all warnings @warning_ignore() annotation don't work with all warnings Jun 22, 2023
@akien-mga akien-mga modified the milestones: 4.1, 4.2 Jun 22, 2023
@paskausks
Copy link

paskausks commented Jul 25, 2023

At least, with INCOMPATIBLE_TERNARY

This works:

@warning_ignore("incompatible_ternary")
foo.bar(
	baz if abc else null
)

while neither of these do not:

# warning-ignore:incompatible_ternary
foo.bar(
	baz if abc else null
)
foo.bar(
	# warning-ignore:incompatible_ternary
	baz if abc else null
)

Also, I noticed, while the comment pattern for warning suppression seems to be documented in https://docs.godotengine.org/en/stable/tutorials/scripting/gdscript/warning_system.html, the annotation pattern isn't.

Windows 11, v4.1.stable.official [9704596]

@Cykyrios
Copy link
Contributor

Cykyrios commented Sep 3, 2023

After the recent #79880 PR, I am getting additional @warning_ignore issues regarding confusable local declarations:

@warning_ignore("confusable_local_declaration")
var my_func := func my_func(param := 0) -> void:
	print(param)
var param := 1
my_func.call(param)

The warning about param being declared below is still shown.

@dalexeev
Copy link
Member

@jice-nospam
Copy link

I confirm warning-ignore doesn't work in 4.1.3 for SHADOWED_GLOBAL_IDENTIFIER :
image

@AdriaandeJongh
Copy link
Contributor

I also found that @warning_ignore("inferred_declaration") doesn't work on the iterator variable of for loops:

@warning_ignore("inferred_declaration")
for prop in __mandatory_properties:
    # etc

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

Successfully merging a pull request may close this issue.