linter does not detect missing space between variable and minus sign

[robotboy655]

x-2 is not detected as a style problem, while x/2 or x+2 are.

I imagine this is tripped up by detection of negative numbers.

Sample code

local function DrawOverlay(test)

	surface.DrawRect( x-2, y-2, w + 4, 2 )
	surface.DrawRect( x/2, y, 2, h )
	surface.DrawRect( x+w, y/2, 2, h )
	surface.DrawRect( ",",x*2, y + h, w + 4, 2 )

	// hehe

end

Output:

Other issues/wishlist:

• Perhaps inconsistent comment styles/operator warnings should point to the previous inconsistent instance in its message, like shadowed variables do, for integration with IDEs.
• Another useful thing could be detecting double spaces where there should be just one
  • i.e. ( test ) => ( test ) or ( a and b ) => ( a and b )
• Empty if statement warning should cover the entire if statement (start/end positions), not just the if itself, just like empty loops do.

[FPtje]

Thanks for the report!

I imagine this is tripped up by detection of negative numbers.

You're right. This part of the linter runs on the lexicon. There it just sees individual tokens, rather than control structures. Specifically, it is part of the "bad sequences" linter, which is just "if you see this sequence of tokens, throw that warning". As such it cannot see the difference between the unary and the binary - operator. For that reason it is left out.

Adding support for that operator is nasty enough that it's probably not worth the effort.

Perhaps inconsistent comment styles/operator warnings should point to the previous inconsistent instance in its message, like shadowed variables do, for integration with IDEs.

This can be done I think. Can that location just be in the message, or should it be encoded in some special way for IDEs to point at it? Message should be fine right?

Another useful thing could be detecting double spaces where there should be just one

Would be nice, but this would false positive on a lot of places where code is aligned. Admittedly that is no longer my style, but people are likely to get annoyed by seeing warnings for things they intended.

Empty if statement warning should cover the entire if statement (start/end positions), not just the if itself, just like empty loops do.

That makes sense 👍

[robotboy655]

This can be done I think. Can that location just be in the message, or should it be encoded in some special way for IDEs to point at it? Message should be fine right?

Just the message is fine, yeah.

Would be nice, but this would false positive on a lot of places where code is aligned. Admittedly that is no longer my style, but people are likely to get annoyed by seeing warnings for things they intended.

Perhaps putting it behind an option flag would be reasonable for this.

[robotboy655]

Just encountered another weird issue - pretty printing this function moves the comment out of the function:

hook.Add( "CalcView", "MyCalcView", function( ply, pos, angles, fov )
	local view = {
		origin = pos,
		angles = angles,
		fov = fov/2
	}

	--return view
end )

If I should open a new issue for this, let me know.

[FPtje]

Yeah, putting comments back where they were sounds super simple, but it has turned out to be the most challenging aspect implemented in Glualint.

[FPtje]

To summarize this issue:

x-2 is not detected as a style problem, while x/2 or x+2 are.

Won't fix, as -x is hard to distinguish from <expression>-x, because the linter works on the lexicon. This is a known limitation.

Perhaps inconsistent comment styles/operator warnings should point to the previous inconsistent instance in its message, like shadowed variables do, for integration with IDEs.

Fixed in #174

Another useful thing could be detecting double spaces where there should be just one
i.e. ( test ) => ( test ) or ( a and b ) => ( a and b )

Won't fix. It's a feature request that would be somewhat nice to have, but I foresee it also being annoying to implement correctly. There might also be a high likelyhood of false positives, e.g. when people align their code. I could then add a config, but it doesn't feel like it's worth the trouble.

Empty if statement warning should cover the entire if statement (start/end positions), not just the if itself, just like empty loops do.

Fixed in 57f6f4f

Just encountered another weird issue - pretty printing this function moves the comment out of the function:

Fixed in #173

That closes the issue!

[FPtje]

Released here: https://github.com/FPtje/GLuaFixer/releases as 1.27.0