Merged
Conversation
hubwriter
previously approved these changes
Oct 13, 2021
docs/codeql/codeql-language-guides/basic-query-for-ruby-code.rst
Outdated
Show resolved
Hide resolved
|
|
||
| The query will take a few moments to return results. When the query completes, the results are displayed below the project name. The query results are listed in two columns, corresponding to the two expressions in the ``select`` clause of the query. The first column corresponds to the expression ``ifexpr`` and is linked to the location in the source code of the project where ``ifexpr`` occurs. The second column is the alert message. | ||
|
|
||
| ➤ `Example query results <https://lgtm.com/query/1214010107827821393/>`__ |
| @@ -0,0 +1,3 @@ | |||
| - `CodeQL queries for Ruby <https://github.com/github/codeql/tree/main/ruby/ql/src>`__ | |||
| - `Example queries for Ruby <https://github.com/github/codeql/tree/main/ruby/ql/examples>`__ | |||
| - `CodeQL library reference for Ruby <https://codeql.github.com/codeql-standard-libraries/ruby/>`__ | |||
Contributor
There was a problem hiding this comment.
links don't currently resolve correctly
|
|
||
| There are now fewer results because ``if`` expressions with an ``else`` branch are no longer included. | ||
|
|
||
| ➤ `See this in the query console <https://lgtm.com/query/6233102733683510530/>`__ |
hubwriter
reviewed
Oct 13, 2021
|
|
||
| import codeql.ruby.CFG | ||
|
|
||
| The CFG reasons about the control flow between statements and expressions, for example whether one expression can |
Contributor
There was a problem hiding this comment.
Is "reason" the right verb here? It seems anthropomorphic to say that the CFG reasons.
hubwriter
reviewed
Oct 13, 2021
hubwriter
reviewed
Oct 13, 2021
hubwriter
reviewed
Oct 13, 2021
hubwriter
reviewed
Oct 13, 2021
hubwriter
reviewed
Oct 13, 2021
hubwriter
reviewed
Oct 13, 2021
hubwriter
reviewed
Oct 13, 2021
Co-authored-by: hubwriter <hubwriter@github.com>
Co-authored-by: hubwriter <hubwriter@github.com>
Co-authored-by: hubwriter <hubwriter@github.com>
Co-authored-by: hubwriter <hubwriter@github.com>
Co-authored-by: hubwriter <hubwriter@github.com>
Co-authored-by: hubwriter <hubwriter@github.com>
Co-authored-by: hubwriter <hubwriter@github.com>
Co-authored-by: hubwriter <hubwriter@github.com>
hmac
reviewed
Oct 18, 2021
| elsif option == "-verbose" | ||
| # nothing to do - handled earlier | ||
| else | ||
| error "unrecognized option" |
Contributor
There was a problem hiding this comment.
This could be simplified, I think.
if option == "-verbose"
# nothing to do - handled earlier
else
error "unrecognised option"
endIt's a bit more artificial, but removes the ambiguity around which if we're talking about.
hmac
reviewed
Oct 18, 2021
| else | ||
| error "unrecognized option" | ||
|
|
||
| In this case, identifying the ``if`` statement with the empty ``then`` branch as redundant is a false positive. One solution to this is to modify the query to ignore empty ``then`` branches if the ``if`` statement has an ``else`` branch. |
Contributor
There was a problem hiding this comment.
This might read a bit better? Avoids "if the if", at least.
Suggested change
| In this case, identifying the ``if`` statement with the empty ``then`` branch as redundant is a false positive. One solution to this is to modify the query to ignore empty ``then`` branches if the ``if`` statement has an ``else`` branch. | |
| In this case, identifying the ``if`` statement with the empty ``then`` branch as redundant is a false positive. One solution to this is to modify the query to select ``if`` statements where both the ``then`` and ``else`` branches are empty. |
hmac
reviewed
Oct 18, 2021
| --------------- | ||
|
|
||
| The abstract syntax tree (AST) represents the elements of the source code organized into a tree. The AST viewer | ||
| in Visual Studio Code shows the AST nodes, including the relevant CodeQL classes and predicates. |
Contributor
There was a problem hiding this comment.
hmac
reviewed
Oct 18, 2021
| @@ -0,0 +1,622 @@ | |||
| .. codeql-library-for-ruby: | |||
Contributor
There was a problem hiding this comment.
Is this file auto-generated at all or is it written entirely by hand? I can see the reference of AST classes getting out of date in the future if we need to manually update it.
Contributor
Author
There was a problem hiding this comment.
It's written by hand - though much of it has been copied+pasted from the source code.
I agree it's likely to get a bit out of date, but the doc is mainly meant as an overview so it doesn't need to be comprehensive.
Contributor
Author
|
@hmac I've addressed your feedback. |
hmac
approved these changes
Oct 20, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The two introductory guides to Ruby analysis:
At the moment the content is still a little rough, but I'd appreciate early feedback on whether this level of detail is what we are looking for.