github / codeql-coding-standards

This repository contains CodeQL queries and libraries which support various Coding Standards.
MIT License
129 stars 59 forks source link

`A2-7-3`: documented type alias to template instantiation considered undocumented #709

Open fjatWbyT opened 1 month ago

fjatWbyT commented 1 month ago

Affected rules

Description

A documented type alias where type-id is a template results in a missing documentation false positive. If the rhs of the (documented) alias declaration does not involve a template, then there is no alert.

Example

```cpp /// @brief Template type template_type description. /// /// @tparam template parameter T. template struct template_type {}; /// @brief Alias to int a_documented_alias. using a_documented_alias = int; /// @brief Alias a2_7_3_fp_alias to instantiated template_type with int template parameter. using a2_7_3_fp_alias = template_type; ``` Running query `UndocumentedUserDefinedType.ql`, AUTOSAR A2-7-3, will evaluate to ``` | declaration of a2_7_3_fp_alias | Declaration entry for user-defined type a2_7_3_fp_alias is missing documentation. | ```
fjatWbyT commented 1 month ago

Hi @lcartey, may I ask whether you have any idea or intuition to the root cause causing this as well as the other similar false positives (e.g. type aliases based on using to templates? Any pointer into the right direction, perhaps a similar issue related to templates previously solved, would be much appreciated.

lcartey commented 3 weeks ago

The CodeQL extractor for C++ walks the AST to determine which Elements are associated with which comments (accessible from Comment.getCommentedElement()). However, that association is currently not stored in the database for using declarations, due to some complexity around the internal model for using declarations in templates.

The appropriate fix here would likely be to provide a special-cased getCommentedElement(..) for using declarations, that looked at line location. However, this would not be able to determine whether a using declaration generated by a macro was correctly commented, because all the elements generated by a macro have the same location. We could exclude these cases as indeterminable in our current model.