Skip to content

Query help style-guide #595

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 3 commits into from
Dec 7, 2018
Merged

Query help style-guide #595

merged 3 commits into from
Dec 7, 2018

Conversation

jf205
Copy link
Contributor

@jf205 jf205 commented Nov 30, 2018

This PR adds a query help style-guide to the docs folder. As with #581, the aim is to help internal/external query writers write informative qhelp which is consistent with the style adopted in the existing qhelp for the built-in queries.

@calumgrant @geoffw0 @aschackmull – thanks for your help on the metadata style-guide. I'd be very grateful for reviews here if possible please.

@jf205 jf205 changed the title add qhelp style-guide Query help style-guide Nov 30, 2018

Section-level elements are used to group the information within the query help file. All query help files should include at least the following section elements, in the order specified:

1. `overview`—a summary of the purpose of the query, including an explanation of what the code does and the how the behavior of the program is affected.
Copy link
Contributor

Choose a reason for hiding this comment

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

This sounds wrong. I think the overview section is more about giving the necessary background information to understand the issue that the query is trying to identify. Also, using a phrase such as "the code" is potentially ambiguous when used like this, as it could refer both to the QL code of the query or the code being analysed.

1. `overview`—a summary of the purpose of the query, including an explanation of what the code does and the how the behavior of the program is affected.
2. `recommendation`—information on how to fix the issue highlighted by the query.
3. `example`—an example of code showing the problem. Where possible, this section should also include a solution to the issue.
4. `references`—relevant references to best practice.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not just references to best practice. That's of course nice to include, if applicable. References to authoritative sources on relevant language semantics should be included, and potentially other relevant references.


Whenever possible, you should include a code example that helps to explain the issue you are highlighting. Any code examples that you include should adhere to the following guidelines:

* The example should be less than 20 lines and ideally it should be runnable. If this is not possible, ensure that the example very clearly illustrates the issue that you are highlighting.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's important that it's runnable. For compiled languages, including enough stuff to make a snippet runnable would unnecessarily bloat the example code.

<p>This example highlights poor coding practice</p>
<sample language = "java" />

Example of poor code
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem properly nested. There's no code in the <sample />. Also, aren't we usually recommending to always have the example code snippets in their own files and include them with the src="file.java" property as below?

Copy link
Contributor Author

@jf205 jf205 Dec 4, 2018

Choose a reason for hiding this comment

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

I suggested the nested code approach after reading the 'Block elements' table in this topic: https://help.semmle.com/wiki/display/SD/Qhelp+files – and since I couldn't find any examples of this approach in the qhelp of our built-in queries the nesting is likely to be incorrect!

Happy to delete this part if specifying src is the preferred style.


## Including references

You should include one or more references to provide further information about the problem that your query is designed to find. References can be of the following types:
Copy link
Contributor

Choose a reason for hiding this comment

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

It should probably be stated that the referenced should be a list formatted with <li> .. </li> for each item.


## Query help example

The following example is a qhelp file for a query from the standard query suite for Java:
Copy link
Contributor

Choose a reason for hiding this comment

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

It should probably be mentioned somewhere above that code words, such as keywords and the like, should be formatted with <code>..</code> as exemplified below.

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.

See inline comments.

@jf205
Copy link
Contributor Author

jf205 commented Dec 4, 2018

@aschackmull thanks for the review. I think I have been able to address your feedback.

1. `overview`—a short summary of the issue that the query identifies, including an explanation of how it could affect the behavior of the program.
2. `recommendation`—information on how to fix the issue highlighted by the query.
3. `example`—an example of code showing the problem. Where possible, this section should also include a solution to the issue.
4. `references`—relevant references, such as authoritative sources on langauage semantics and best practice.
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: s/langauage/language/

@aschackmull
Copy link
Contributor

One final comment re. a typo, otherwise LGTM.

Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

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

👍 from me.

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

Successfully merging this pull request may close these issues.

3 participants