Skip to content

Java: Update Annotation predicate examples in language guide #17026

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

Merged
merged 1 commit into from
Jul 24, 2024

Conversation

Marcono1234
Copy link
Contributor

#6246 added new predicates and deprecated some of the existing predicates, including getAValue() which is currently used in the example.

Please let me know if the new documentation text is too verbose or if I should adjust it in some way.

}
}

Here, we use ``getAValue()`` to retrieve any annotation value: in fact, annotation type ``SuppressWarnings`` only has a single annotation element, so every ``@SuppressWarnings`` annotation only has a single annotation value. Then, we ensure that it is a literal, obtain its string value using ``getLiteral``, and check whether it contains the string ``deprecation`` using a regular expression match.
Here, we use ``getAStringArrayValue("value")`` to retrieve any of the suppressed warnings: ``@SuppressWarnings`` defines the warnings to suppress using the annotation element named ``value`` of type ``String[]``, and ``getAStringArrayValue`` retrieves all of the array values; the CodeQL class ``Annotation`` also has similar convenience predicates for the other possible annotation element types. Afterwards we check whether one of the values is the string ``deprecation`` using a regular expression match.

For real-world use, this check would have to be generalized a bit: for example, the OpenJDK Java compiler allows ``@SuppressWarnings("all")`` annotations to suppress all warnings. We may also want to make sure that ``deprecation`` is matched as an entire word, and not as part of another word, by changing the regular expression to ``".*\\bdeprecation\\b.*"``.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side note: Not completely sure why a regex match is performed here in the first place. Maybe the original author thought the suppressions were a single String instead of a String[]?

}
}

Here, we use ``getAValue()`` to retrieve any annotation value: in fact, annotation type ``SuppressWarnings`` only has a single annotation element, so every ``@SuppressWarnings`` annotation only has a single annotation value. Then, we ensure that it is a literal, obtain its string value using ``getLiteral``, and check whether it contains the string ``deprecation`` using a regular expression match.
Here, we use ``getAStringArrayValue("value")`` to retrieve any of the suppressed warnings: ``@SuppressWarnings`` defines the warnings to suppress using the annotation element named ``value`` of type ``String[]``, and ``getAStringArrayValue`` retrieves all of the array values; the CodeQL class ``Annotation`` also has similar convenience predicates for the other possible annotation element types. Afterwards we check whether one of the values is the string ``deprecation`` using a regular expression match.

For real-world use, this check would have to be generalized a bit: for example, the OpenJDK Java compiler allows ``@SuppressWarnings("all")`` annotations to suppress all warnings. We may also want to make sure that ``deprecation`` is matched as an entire word, and not as part of another word, by changing the regular expression to ``".*\\bdeprecation\\b.*"``.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side note:

the OpenJDK Java compiler allows @SuppressWarnings("all") annotations to suppress all warnings

This seems to be incorrect, javac does not support the value "all" but many IDEs support it, see JDK-7141469.

Copy link
Contributor

@aschackmull aschackmull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@aschackmull aschackmull merged commit 9f79a39 into github:main Jul 24, 2024
3 checks passed
@Marcono1234 Marcono1234 deleted the patch-1 branch July 27, 2024 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants