Java: Add query to detect special characters in string literals #19875

tamasvajk · 2025-06-25T11:34:43Z

This pull request introduces a new query to detect non-explicit control and whitespace characters in Java literals, improving code readability and reducing potential bugs caused by invisible or hard-to-recognize characters.

Manually testing autofix works, the special characters are replaced with \u0....

owen-mc · 2025-07-01T13:52:41Z

...icitControlAndWhitespaceCharsInLiterals/NonExplicitControlAndWhitespaceCharsInLiterals.qlref

@@ -0,0 +1 @@
+Violations of Best Practice/SpecialCharactersInLiterals/NonExplicitControlAndWhitespaceCharsInLiterals.ql


Did you consider an inline expectations test?

Yes, I quickly turned it into an inline expectations test, but it failed, I think due to the fact that the strings contain multiple special characters. So then I reverted it, and kept it as it is.

owen-mc · 2025-07-01T13:51:55Z

... Best Practice/SpecialCharactersInLiterals/NonExplicitControlAndWhitespaceCharsInLiterals.ql

+ *       correctness
+ *       maintainability
+ *       readability


Just checking: you believe this matches both the correctness and readability categories?

I think readability is definitely okay. correctness might/could be challenged. What do you think?

I think it can belong to both. But between maintainability and reliability, you think it's better to choose maintainability, because readability is definitely an issue whereas correctness is only occasionally an issue?

I haven't given it this much thought. I simply thought maintainability goes together with readability. I think we could remove the correctness tag, and then it fully matches the guidelines, of having one top level tag and one subcategory from that tag.

Eh, I think it's fine as it is actually.

Hmm, I just read the description of this PR and it says that we shouldn't use subcategories from different top-level categories. I will discuss the issue on that PR. Maybe wait until that discussion is complete to see if we should remove correctness.

You don't need to remove correctness.

I already removed it. I think it's okay if we keep the tags to a minimum for the time being.

owen-mc · 2025-07-01T13:52:41Z

...icitControlAndWhitespaceCharsInLiterals/NonExplicitControlAndWhitespaceCharsInLiterals.qlref

@@ -0,0 +1 @@
+Violations of Best Practice/SpecialCharactersInLiterals/NonExplicitControlAndWhitespaceCharsInLiterals.ql


Did you consider an inline expectations test?

owen-mc · 2025-07-02T09:31:55Z

... Best Practice/SpecialCharactersInLiterals/NonExplicitControlAndWhitespaceCharsInLiterals.md

+- [Unicode Control Characters](https://www.unicode.org/charts/PDF/U0000.pdf).
+- [Unicode C0 control codes](https://en.wikipedia.org/wiki/C0_and_C1_control_codes).
+- [Unicode characters with property "WSpace=yes" or "White_Space=yes"](https://en.wikipedia.org/wiki/Unicode_character_property#Whitespace).
+- [Java String Literals](https://docs.oracle.com/javase/tutorial/java/data/characters.html).
+- [Java Class Charset](https://docs.oracle.com/javase/8/docs/api///?java/nio/charset/Charset.html).


It's a bit pedantic, but these don't follow the style guide for references. Examples:

- Java API Specification: [Pattern.MULTILINE](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/regex/Pattern.html#MULTILINE) - Wikipedia: [Template method pattern](https://en.wikipedia.org/wiki/Template_method_pattern).

subatoi

Looks good—only trivial comments 👍

... Best Practice/SpecialCharactersInLiterals/NonExplicitControlAndWhitespaceCharsInLiterals.md

tamasvajk · 2025-07-03T09:17:08Z

@subatoi Thank you, I've applied your suggestions in 3d2a3a3.

owen-mc · 2025-07-03T09:34:02Z

... Best Practice/SpecialCharactersInLiterals/NonExplicitControlAndWhitespaceCharsInLiterals.ql

+ * @tags quality
+ *       maintainability
+ *       readability


@michaelnebel and I concluded that you can keep correctness, btw.

owen-mc · 2025-07-08T10:29:18Z

I just noticed that this is @precision medium and hence not in the default quality suite. Does it give lots of FPs? (I guess for quality that means results that users won't care about.)

tamasvajk · 2025-07-08T11:08:13Z

I just noticed that this is @precision medium and hence not in the default quality suite. Does it give lots of FPs? (I guess for quality that means results that users won't care about.)

I think we can increase the precision. There are 139 alerts on the top 1000 MRVA repos.

owen-mc

Glad this will reach more people. It isn't about the number of alerts, but the probability that an alert will be a TP/FP. But presumably since you went for very-high you feel pretty good about the FP rate from the alerts that you looked at.

This reverts commit 92685e6.

github-actions bot added documentation Java labels Jun 25, 2025

github-advanced-security bot found potential problems Jun 25, 2025

View reviewed changes

tamasvajk force-pushed the quality/spec_chars branch 3 times, most recently from e1b966e to 9db9200 Compare June 25, 2025 12:15

tamasvajk added the no-change-note-required This PR does not need a change note label Jun 25, 2025

tamasvajk force-pushed the quality/spec_chars branch 2 times, most recently from eeaaa87 to 834cb1b Compare July 1, 2025 09:38

tamasvajk marked this pull request as ready for review July 1, 2025 12:07

tamasvajk requested a review from a team as a code owner July 1, 2025 12:07

owen-mc reviewed Jul 1, 2025

View reviewed changes

tamasvajk added the ready-for-doc-review This PR requires and is ready for review from the GitHub docs team. label Jul 2, 2025

owen-mc reviewed Jul 2, 2025

View reviewed changes

owen-mc mentioned this pull request Jul 2, 2025

Ql4ql: Quality query tagging. #19931

Open

subatoi previously approved these changes Jul 3, 2025

View reviewed changes

tamasvajk dismissed subatoi’s stale review via 3d2a3a3 July 3, 2025 09:15

owen-mc previously approved these changes Jul 3, 2025

View reviewed changes

owen-mc reviewed Jul 3, 2025

View reviewed changes

tamasvajk dismissed owen-mc’s stale review via 4fcbdc6 July 3, 2025 09:38

tamasvajk force-pushed the quality/spec_chars branch from 4fcbdc6 to 14a91dc Compare July 3, 2025 14:05

owen-mc previously approved these changes Jul 8, 2025

View reviewed changes

tamasvajk dismissed owen-mc’s stale review via ddfd774 July 8, 2025 11:11

tamasvajk force-pushed the quality/spec_chars branch from 14a91dc to ddfd774 Compare July 8, 2025 11:11

owen-mc previously approved these changes Jul 8, 2025

View reviewed changes

tamasvajk added 2 commits July 8, 2025 13:28

Java: Add query to detect special characters in string literals

09a2aee

Exclude Kotlin files

fd8b37c

tamasvajk added 5 commits July 8, 2025 13:28

Adjust references in query doc

a0c9c98

Adjust query tags

15de398

Improve query documentation

c4def10

Revert "Adjust query tags"

d16570b

This reverts commit 92685e6.

Adjust query precision

ccbf705

tamasvajk dismissed owen-mc’s stale review via ccbf705 July 8, 2025 11:31

tamasvajk force-pushed the quality/spec_chars branch from ddfd774 to ccbf705 Compare July 8, 2025 11:31

owen-mc approved these changes Jul 8, 2025

View reviewed changes

tamasvajk merged commit 4f1ca21 into github:main Jul 8, 2025
18 checks passed

		@@ -0,0 +1 @@
		Violations of Best Practice/SpecialCharactersInLiterals/NonExplicitControlAndWhitespaceCharsInLiterals.ql

Java: Add query to detect special characters in string literals #19875

Java: Add query to detect special characters in string literals #19875

Uh oh!

Conversation

tamasvajk commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Check warning

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tamasvajk Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

owen-mc Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

subatoi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tamasvajk commented Jul 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

owen-mc commented Jul 8, 2025

Uh oh!

tamasvajk commented Jul 8, 2025

Uh oh!

owen-mc left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tamasvajk commented Jun 25, 2025 •

edited

Loading

tamasvajk Jul 2, 2025 •

edited

Loading

owen-mc Jul 2, 2025 •

edited

Loading

owen-mc left a comment •

edited

Loading