Skip to content

QL: add query detecting consistent casing of names #10209

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 2 commits into from
Sep 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions ql/ql/src/codeql_ql/style/NodeName.qll
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ string getName(AstNode node, string kind) {
or
// not including CharPreds or db relations. The remaining are: classlessPredicate, classPredicate, newTypeBranch.
result = node.(ClasslessPredicate).getName() and
kind = "classlessPredicate"
kind = "predicate"
or
result = node.(ClassPredicate).getName() and
kind = "classPredicate"
kind = "predicate"
or
result = node.(NewTypeBranch).getName() and
kind = "newtypeBranch"
kind = "newtype"
or
result = node.(NewType).getName() and
kind = "newtype"
Expand All @@ -28,5 +28,5 @@ string getName(AstNode node, string kind) {
or
result = node.(Module).getName() and kind = "module"
or
result = node.(Import).importedAs() and kind = "import"
result = node.(Import).importedAs() and kind = "module"
}
63 changes: 63 additions & 0 deletions ql/ql/src/queries/style/ConsistentCasing.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/**
* @name Names only differing by case
* @description Names only differing by case can cause confusion.
* @kind problem
* @problem.severity warning
* @id ql/names-only-differing-by-case
* @tags correctness
* maintainability
* @precision high
*/

import ql
import codeql_ql.style.AcronymsShouldBeCamelCaseQuery as Acronyms
private import codeql_ql.style.NodeName as NodeName

string getName(AstNode node, string kind, YAML::QLPack pack) {
result = NodeName::getName(node, kind) and
node.getLocation().getFile() = pack.getAFileInPack() and
not node.hasAnnotation("deprecated") and // deprecated nodes are not interesting
not node.getLocation().getFile().getBaseName() = "TreeSitter.qll" and // auto-generated, is sometimes bad.
not node.getLocation().getFile().getExtension() = "ql" // .ql files cannot be imported, so no risk of conflict
}

// get better join-order by getting all the relevant information in a single utility predicate
pragma[nomagic]
string getNameAndPack(AstNode node, string kind, string lower, YAML::QLPack pack) {
result = getName(node, kind, pack) and lower = result.toLowerCase()
}

predicate problem(string name1, AstNode node1, string name2, string kind) {
exists(string lower, YAML::QLPack pack1, YAML::QLPack pack2 |
name1 = getNameAndPack(node1, kind, lower, pack1) and
name1.length() >= 2 and
pack2 = pack1.getADependency*() and
name2 = getNameAndPack(_, kind, lower, pack2) and // TODO: Get if it's a dependency pack in the alert-message.
name1 != name2 and
kind != "variable" // these are benign, and plentyful...
)
}

string prettyPluralKind(string kind) {
kind = "class" and result = "classes"
or
kind = "classlessPredicate" and result = "predicates"
or
kind = "classPredicate" and result = "class predicates"
or
kind = "newtypeBranch" and result = "newtype branches"
or
kind = "newtype" and result = "newtypes"
or
kind = "variable" and result = "variables"
or
kind = "field" and result = "fields"
or
kind = "module" and result = "modules"
}

from string name1, AstNode node, string name2, string kind
where problem(name1, node, name2, kind) and not name1.toLowerCase() = "geturl"
select node,
name1 + " is only different by casing from " + name2 + " that is used elsewhere for " +
prettyPluralKind(kind) + "."
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
| Test.qll:1:1:3:3 | QLDoc | This comment contains the common misspelling 'mispelled', which should instead be 'misspelled'. |
| Test.qll:4:7:4:26 | Class PublicallyAccessible | This class name contains the common misspelling 'publically', which should instead be 'publicly'. |
| Test.qll:5:3:5:20 | FieldDecl | This field name contains the common misspelling 'occurences', which should instead be 'occurrences'. |
| Test.qll:10:13:10:23 | ClassPredicate hasAgrument | This classPredicate name contains the common misspelling 'agrument', which should instead be 'argument'. |
| Test.qll:10:13:10:23 | ClassPredicate hasAgrument | This predicate name contains the common misspelling 'agrument', which should instead be 'argument'. |
| Test.qll:13:1:16:3 | QLDoc | This comment contains the non-US spelling 'colour', which should instead be 'color'. |
| Test.qll:17:7:17:17 | Class AnalysedInt | This class name contains the non-US spelling 'analysed', which should instead be 'analyzed'. |