diff --git a/.editorconfig b/.editorconfig index 413e18ca658..268bd5a7ecb 100644 --- a/.editorconfig +++ b/.editorconfig @@ -1,2 +1,2 @@ -[*.{ql,qll,qlref,dbscheme,qhelp,html,js,mjs,ts,json,yml,c,cpp,h,hpp}] +[*] end_of_line = lf diff --git a/.gitattributes b/.gitattributes index d945c85a015..bd6bfa96bcc 100644 --- a/.gitattributes +++ b/.gitattributes @@ -16,12 +16,25 @@ *.dbscheme eol=lf *.qhelp eol=lf *.html eol=lf +*.htm eol=lf +*.xhtml eol=lf +*.xhtm eol=lf *.js eol=lf *.mjs eol=lf *.ts eol=lf *.json eol=lf *.yml eol=lf +*.yaml eol=lf *.c eol=lf *.cpp eol=lf *.h eol=lf *.hpp eol=lf +*.md eol=lf +*.stats eol=lf +*.xml eol=lf +*.sh eol=lf +*.pl eol=lf +*.java eol=lf +*.cs eol=lf +*.py eol=lf +*.lua eol=lf diff --git a/change-notes/1.19/analysis-cpp.md b/change-notes/1.19/analysis-cpp.md index 7fcf1fd234c..5a3340603c8 100644 --- a/change-notes/1.19/analysis-cpp.md +++ b/change-notes/1.19/analysis-cpp.md @@ -1,20 +1,20 @@ -# Improvements to C/C++ analysis - -## General improvements - -## New queries - -| **Query** | **Tags** | **Purpose** | -|-----------------------------|-----------|--------------------------------------------------------------------| -| *@name of query (Query ID)* | *Tags* |*Aim of the new query and whether it is enabled by default or not* | - -## Changes to existing queries - -| **Query** | **Expected impact** | **Change** | -|----------------------------|------------------------|------------------------------------------------------------------| -| Resource not released in destructor | Fewer false positive results | Placement new is now excluded from the query. | - - -## Changes to QL libraries - -* Added a hash consing library for structural comparison of expressions. +# Improvements to C/C++ analysis + +## General improvements + +## New queries + +| **Query** | **Tags** | **Purpose** | +|-----------------------------|-----------|--------------------------------------------------------------------| +| *@name of query (Query ID)* | *Tags* |*Aim of the new query and whether it is enabled by default or not* | + +## Changes to existing queries + +| **Query** | **Expected impact** | **Change** | +|----------------------------|------------------------|------------------------------------------------------------------| +| Resource not released in destructor | Fewer false positive results | Placement new is now excluded from the query. | + + +## Changes to QL libraries + +* Added a hash consing library for structural comparison of expressions. diff --git a/cpp/ql/src/plugin.xml b/cpp/ql/src/plugin.xml index e08a29947a6..d7036c7c347 100644 --- a/cpp/ql/src/plugin.xml +++ b/cpp/ql/src/plugin.xml @@ -1,16 +1,16 @@ - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + diff --git a/cpp/ql/src/tools/instantiate_templates.py b/cpp/ql/src/tools/instantiate_templates.py index 0beeeed20d8..d549a85f82d 100644 --- a/cpp/ql/src/tools/instantiate_templates.py +++ b/cpp/ql/src/tools/instantiate_templates.py @@ -1,137 +1,137 @@ -import sys -import os.path -import glob -import re -import json - -BEGIN_TEMPLATE = re.compile(r"^/\*template\s*$") -END_TEMPLATE = re.compile(r"^\*/\s*$") - -def expand_template_params(args, param_arg_map): - '''Given a list of template arguments that may reference template parameters - of the current template, return a new list of template arguments with each - parameter use replaced with the appropriate fully-qualified argument for - that parameter.''' - result = [] - for arg in args: - if arg in param_arg_map: - result.append(param_arg_map[arg]) - else: - result.append(arg) - - return result - -def find_instantiation(module, args, templates): - '''Given a template module and a set of template arguments, find the module - name of the instantiation of that module with those arguments.''' - template = templates[module] - for instantiation in template["template_def"]["instantiations"]: - if instantiation["args"] == args: - return instantiation["name"] - return None - -def instantiate_template(template, instantiation, root, templates): - '''Create a single instantiation of a template.''' - template_def = template["template_def"] - output_components = instantiation["name"].split(".") - output_path = root - for component in output_components: - output_path = os.path.join(output_path, component) - output_path = output_path + ".qll" - with open(output_path, "w") as output: - output.write( -""" -/* - * THIS FILE IS AUTOMATICALLY GENERATED FROM '%s'. - * DO NOT EDIT MANUALLY. - */ - -""" % (template["name"].replace(".", "/") + ".qllt") - ) - param_arg_map = {} - for param_index in range(len(template_def["params"])): - param = template_def["params"][param_index] - arg = instantiation["args"][param_index] - output.write("private import %s as %s // Template parameter\n" % (arg, param)) - param_arg_map[param] = arg - for import_record in template_def["imports"]: - if "access" in import_record: - output.write(import_record["access"] + " ") - imported_module = find_instantiation(import_record["module"], - expand_template_params(import_record["args"], param_arg_map), templates) - output.write("import %s // %s<%s>\n" % - ( - imported_module, - import_record["module"], - ", ".join(import_record["args"]) - ) - ) - - output.writelines(template_def["body_lines"]) - -def generate_instantiations(template, root, templates): - '''Create a .qll source file for each instantiation of the specified template.''' - template_def = template["template_def"] - if "instantiations" in template_def: - for instantiation in template_def["instantiations"]: - instantiate_template(template, instantiation, root, templates) - -def read_template(template_path, module_name): - '''Read a .qllt template file from template_path, using module_name as the - fully qualified name of the module.''' - with open(template_path) as input: - in_template = False - template_text = "" - template_def = None - body_lines = [] - for line in iter(input): - if in_template: - if END_TEMPLATE.match(line): - template_def = json.loads(template_text) - in_template = False - else: - template_text += line - else: - if BEGIN_TEMPLATE.match(line) and not template_def: - in_template = True - else: - body_lines.append(line) - - if template_def: - template_def["body_lines"] = body_lines - - result = { "name": module_name } - if template_def: - result["template_def"] = template_def - return result - -def module_name_from_path_impl(path): - (head, tail) = os.path.split(path) - if head == "": - return tail - else: - return module_name_from_path(head) + "." + tail - -def module_name_from_path(path): - '''Compute the fully qualified name of a module from the path of its .qll[t] - file. The path should be relative to the library root.''' - (module_root, ext) = os.path.splitext(path) - return module_name_from_path_impl(module_root) - -def main(): - templates = {} - - root = sys.argv[1] - for template_path in glob.glob(os.path.join(root, "**\\*.qllt"), recursive = True): - print(template_path) - module_name = module_name_from_path(os.path.relpath(template_path, root)) - print(module_name) - template = read_template(template_path, module_name) - templates[template["name"]] = template - - for name, template in templates.items(): - if "template_def" in template: - generate_instantiations(template, root, templates) - -if __name__ == "__main__": - main() +import sys +import os.path +import glob +import re +import json + +BEGIN_TEMPLATE = re.compile(r"^/\*template\s*$") +END_TEMPLATE = re.compile(r"^\*/\s*$") + +def expand_template_params(args, param_arg_map): + '''Given a list of template arguments that may reference template parameters + of the current template, return a new list of template arguments with each + parameter use replaced with the appropriate fully-qualified argument for + that parameter.''' + result = [] + for arg in args: + if arg in param_arg_map: + result.append(param_arg_map[arg]) + else: + result.append(arg) + + return result + +def find_instantiation(module, args, templates): + '''Given a template module and a set of template arguments, find the module + name of the instantiation of that module with those arguments.''' + template = templates[module] + for instantiation in template["template_def"]["instantiations"]: + if instantiation["args"] == args: + return instantiation["name"] + return None + +def instantiate_template(template, instantiation, root, templates): + '''Create a single instantiation of a template.''' + template_def = template["template_def"] + output_components = instantiation["name"].split(".") + output_path = root + for component in output_components: + output_path = os.path.join(output_path, component) + output_path = output_path + ".qll" + with open(output_path, "w") as output: + output.write( +""" +/* + * THIS FILE IS AUTOMATICALLY GENERATED FROM '%s'. + * DO NOT EDIT MANUALLY. + */ + +""" % (template["name"].replace(".", "/") + ".qllt") + ) + param_arg_map = {} + for param_index in range(len(template_def["params"])): + param = template_def["params"][param_index] + arg = instantiation["args"][param_index] + output.write("private import %s as %s // Template parameter\n" % (arg, param)) + param_arg_map[param] = arg + for import_record in template_def["imports"]: + if "access" in import_record: + output.write(import_record["access"] + " ") + imported_module = find_instantiation(import_record["module"], + expand_template_params(import_record["args"], param_arg_map), templates) + output.write("import %s // %s<%s>\n" % + ( + imported_module, + import_record["module"], + ", ".join(import_record["args"]) + ) + ) + + output.writelines(template_def["body_lines"]) + +def generate_instantiations(template, root, templates): + '''Create a .qll source file for each instantiation of the specified template.''' + template_def = template["template_def"] + if "instantiations" in template_def: + for instantiation in template_def["instantiations"]: + instantiate_template(template, instantiation, root, templates) + +def read_template(template_path, module_name): + '''Read a .qllt template file from template_path, using module_name as the + fully qualified name of the module.''' + with open(template_path) as input: + in_template = False + template_text = "" + template_def = None + body_lines = [] + for line in iter(input): + if in_template: + if END_TEMPLATE.match(line): + template_def = json.loads(template_text) + in_template = False + else: + template_text += line + else: + if BEGIN_TEMPLATE.match(line) and not template_def: + in_template = True + else: + body_lines.append(line) + + if template_def: + template_def["body_lines"] = body_lines + + result = { "name": module_name } + if template_def: + result["template_def"] = template_def + return result + +def module_name_from_path_impl(path): + (head, tail) = os.path.split(path) + if head == "": + return tail + else: + return module_name_from_path(head) + "." + tail + +def module_name_from_path(path): + '''Compute the fully qualified name of a module from the path of its .qll[t] + file. The path should be relative to the library root.''' + (module_root, ext) = os.path.splitext(path) + return module_name_from_path_impl(module_root) + +def main(): + templates = {} + + root = sys.argv[1] + for template_path in glob.glob(os.path.join(root, "**\\*.qllt"), recursive = True): + print(template_path) + module_name = module_name_from_path(os.path.relpath(template_path, root)) + print(module_name) + template = read_template(template_path, module_name) + templates[template["name"]] = template + + for name, template in templates.items(): + if "template_def" in template: + generate_instantiations(template, root, templates) + +if __name__ == "__main__": + main() diff --git a/csharp/ql/src/plugin.xml b/csharp/ql/src/plugin.xml index f44185688ff..532f6c121ad 100644 --- a/csharp/ql/src/plugin.xml +++ b/csharp/ql/src/plugin.xml @@ -1,16 +1,16 @@ - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + diff --git a/csharp/ql/test/library-tests/standalone/controlflow/ControlFlow.cs b/csharp/ql/test/library-tests/standalone/controlflow/ControlFlow.cs index 56a6e5121cb..1dedc955c33 100644 --- a/csharp/ql/test/library-tests/standalone/controlflow/ControlFlow.cs +++ b/csharp/ql/test/library-tests/standalone/controlflow/ControlFlow.cs @@ -1,14 +1,14 @@ -// semmle-extractor-options: --standalone - -using System; - -class Cfg -{ - void F() - { - var v = new InvalidType(); - Debug.Assert(v.a.b, "This is true"); - - new CounterCreationData() { CounterHelp = string.Empty, CounterType = v.Type }; - } -} +// semmle-extractor-options: --standalone + +using System; + +class Cfg +{ + void F() + { + var v = new InvalidType(); + Debug.Assert(v.a.b, "This is true"); + + new CounterCreationData() { CounterHelp = string.Empty, CounterType = v.Type }; + } +} diff --git a/docs/ql-style-guide.md b/docs/ql-style-guide.md index ad30770944d..2e2f8eb39f7 100644 --- a/docs/ql-style-guide.md +++ b/docs/ql-style-guide.md @@ -1,417 +1,417 @@ -# QL Style Guide - -## Introduction - -This document describes how to format the QL code you contribute to this repository. It covers aspects such as layout, white-space, naming and documentation. Adhering to consistent standards makes code easier to read and maintain. Of course, these are only guidelines, and can be overridden as the need arises on a case-by-case basis. Where existing code deviates from these guidelines, prefer consistency with the surrounding code. - -Words in *italic* are defined in the [Glossary](#glossary). - -## Indentation -1. *Always* use 2 spaces for indentation. -1. *Always* indent: - - The *body* of a module, newtype, class or predicate - - The second and subsequent lines after you use a line break to split a long line - - The *body* of a `from`, `where` or `select` clause where it spans multiple lines - - The *body* of a *quantifier* that spans multiple lines - - -### Examples - -```ql -module Helpers { - /** ... */ - class X ... { - /** ... */ - int getNumberOfChildren () { - result = count(int child | - exists(this.getChild(child)) - ) - } - } -} -``` - -```ql -from Call c, string reason -where isDeprecated(c, reason) -select c, "This call to '$@' is deprecated because " + reason + ".", - c.getTarget(), c.getTarget().getName() -``` - -## Line breaks -1. Use UNIX line endings. -1. Lines *must not* exceed 100 characters. -1. Long lines *should* be split with a line break, and the following lines *must* be indented one level until the next "regular" line break. -1. There *should* be a single blank line: - - Between the file documentation and the first `import` - - Before each declaration, except for the first declaration in a *body* - - Before the `from`-`where`-`select` section in a query file -1. *Avoid* two or more adjacent blank lines. -1. There *must* be a new line after the *annotations* `cached`, `pragma`, `language` and `bindingset`. Other *annotations* do not have a new line. -1. There *should not* be additional blank lines within a predicate. -1. There *may* be a new line: - - Immediately after the `from`, `where` or `select` keywords in a query. - - Immediately after `if`, `then`, or `else` keywords. The `then` and `else` parts *should* be consistent. -1. *Avoid* other line breaks in declarations, other than to break long lines. -1. When operands of *binary operators* span two lines, the operator *should* be placed at the end of the first line. - -### Examples - -```ql -cached -private int getNumberOfParameters() { - ... -} -``` - -```ql -predicate methodStats(string qualifiedName, string name, - int numberOfParameters, int numberOfStatements, int numberOfExpressions, - int linesOfCode, int nestingDepth, int numberOfBranches) { - ... -} -``` - -```ql -from Method main -where main.getName() = "Main" -select main, "This is the program entry point." -``` - -```ql -from Method main -where - main.getName() = "Main" and - main.getNumberOfParameters() = 0 -select main, "Main method has no parameters." -``` - -```ql - if x.isPublic() - then result = "public" - else result = "private" -``` - -```ql - if x.isPublic() then - result = "public" - else - result = "private" -``` - -```ql - if - x.isPublic() - then - result = "public" - else - result = "private" -``` - -## Braces -1. Braces follow [Stroustrup](https://en.wikipedia.org/wiki/Indentation_style#Variant:_Stroustrup) style. The opening `{` *must* be placed at the end of the preceding line. -1. The closing `}` *must* be placed on its own line, indented to the outer level, or be on the same line as the opening `{`. -1. Braces of empty blocks *may* be placed on a single line, with a single space separating the braces. -1. Short predicates, not exceeding the maximum line width, *may* be placed on a single line, with a space following the opening brace and preceding the closing brace. - -### Examples - -```ql -class ThrowException extends ThrowExpr { - Foo() { - this.getTarget() instanceof ExceptionClass - } - - override string toString() { result = "Throw Exception" } -} -``` - -## Spaces -1. There *must* be a space or line break: - - Surrounding each `=` and `|` - - After each `,` -1. There *should* be a space or line break: - - Surrounding each *binary operator*, which *must* be balanced - - Surrounding `..` in a range - - Exceptions to this may be made to save space or to improve readability. -1. *Avoid* other spaces, for example: - - After a *quantifier/aggregation* keyword - - After the predicate name in a *call* - - Inside brackets used for *calls*, single-line quantifiers, and parenthesised formulas - - Surrounding a `.` - - Inside the opening or closing `[ ]` in a range expression - - Inside casts `a.(X)` -1. *Avoid* multiple spaces, except for indentation, and *avoid* additional indentation to align formulas, parameters or arguments. -1. *Do not* put whitespace on blank lines, or trailing on the end of a line. -1. *Do not* use tabs. - - -### Examples - -```ql -cached -private predicate foo(Expr e, Expr p) { - exists(int n | - n in [0 .. 1] | - e = p.getChild(n + 1) - ) -} -``` - -## Naming -1. Use [PascalCase](http://wiki.c2.com/?PascalCase) for: - - `class` names - - `module` names - - `newtype` names -1. Use [camelCase](https://en.wikipedia.org/wiki/Camel_case) for: - - Predicate names - - Variable names -1. Newtype predicate names *should* begin with `T`. -1. Predicates that have a result *should* be named `get...` -1. Predicates that can return multiple results *should* be named `getA...` or `getAn...` -1. Predicates that don't have a result or parameters *should* be named `is...` or `has...` -1. *Avoid* underscores in names. -1. *Avoid* short or single-letter names for classes, predicates and fields. -1. Short or single letter names for parameters and *quantifiers* *may* be used provided that they are sufficiently clear. -1. Use names as they are used in the target-language specification. -1. Use American English. - -### Examples - -```ql -/** ... */ -predicate calls(Callable caller, Callable callee) { - ... -} -``` - -```ql -/** ... */ -class Type extends ... { - /** ... */ - string getName() { ... } - - /** ... */ - predicate declares(Member m) { ... } - - /** ... */ - predicate isGeneric() { ... } - - /** ... */ - Type getTypeParameter(int n) { ... } - - /** ... */ - Type getATypeParameter() { ... } -} -``` - -## Documentation - -General requirements: - -1. Documentation *must* adhere to the [QLDoc specification](https://help.semmle.com/QL/QLDocSpecification.html). -1. Use `/** ... */` for documentation, even for single line comments. -1. For single-line documentation, the `/**` and `*/` are written on the same line as the comment. -1. For multi-line documentation, the `/**` and `*/` are written on separate lines. There is a `*` preceding each comment line, aligned on the first `*`. -1. Use full sentences, with capital letters and full stops. -1. Use American English. -1. Documentation comments *should* be appropriate for users of the code. -1. Documentation for maintainers of the code *must* use normal comments. - -Documentation for specific items: - -1. Public declarations *must* be documented. -1. Non-public declarations *should* be documented. -1. Declarations in query files *should* be documented. -1. Library files (`.qll` files) *should* be have a documentation comment at the top of the file. -1. Query files, except for tests, *must* have a QLDoc query documentation comment at the top of the file. -1. Predicates that do not have a result *should* be documented `/** Holds if ... */` -1. Predicates that have a result *should* be documented `/** Gets ... */` -1. All predicate parameters *should* be referred to in the predicate documentation. -1. Reference names, such as types and parameters, using backticks `` ` ``. -1. Give examples of code in the target language, enclosed in ```` ``` ```` or `` ` ``. -1. Classes *should* be documented in the singular, for example `/* An expression. */` -1. Where a class denotes a generic concept with subclasses, list those subclasses. -1. Declarations that are deprecated *should* be documented as `DEPRECATED: ...` -1. Declarations that are for internal use *should* be documented as `INTERNAL: Do not use`. - -### Examples - -```ql -/** Provides logic for determining constant expressions. */ -``` - -```ql -/** - * Holds if the qualifier of this call has type `qualifierType`. - * `isExactType` indicates whether the type is exact, that is, whether - * the qualifier is guaranteed not to be a subtype of `qualifierType`. - */ -``` -```ql -/** - * A delegate declaration, for example - * ``` - * delegate void Logger(string text); - * ``` - */ -class Delegate extends ... -``` - -```ql -/** - * An element that can be called. - * - * Either a method (`Method`), a constructor (`Constructor`), a destructor - * (`Destructor`), an operator (`Operator`), an accessor (`Accessor`), - * an anonymous function (`AnonymousFunctionExpr`), or a local function - * (`LocalFunction`). - */ -class Callable extends ... -``` - -```ql -/** DEPRECATED: Use `getAnExpr()` instead. */ -deprecated Expr getInitializer() -``` - -```ql -/** - * INTERNAL: Do not use. - */ -``` - -## Formulas -1. *Prefer* one *conjunct* per line. -1. Write the `and` at the end of the line. This also applies in `where` clauses. -1. *Prefer* to write the `or` keyword on its own line. -1. The `or` keyword *may* be written at the end of a line, or within a line, provided that it has no unparenthesised `and` operands. -1. Single-line formulas *may* be used in order to save space or add clarity, particularly in the *body* of a *quantifier/aggregation*. -1. *Always* use brackets to clarify the precedence of: - - `implies` - - `if`-`then`-`else` -1. Parenthesised formulas *can* be written: - - Within a single line. There *should not* be an additional space following the opening parenthesis or preceding the closing parenthesis. - - Spanning multiple lines. The opening parenthesis *should* be placed at the end of the preceding line, the body should be indented one level, and the closing bracket should be placed on a new line at the outer indentation. -1. *Quantifiers/aggregations* *can* be written: - - Within a single line. In this case, there is no space to the inside of the parentheses, or after the quantifier keyword. - - Across multiple lines. In this case, type declarations are on the same line as the quantifier, the `|` *may* be at the end of the line, or *may* be on its own line, and the body of the quantifier *must* be indented one level. The closing `)` is written on a new line, at the outer indentation. -1. `if`-`then`-`else` *can* be written: - - On a single line - - With the *body* after the `if`/`then`/`else` keyword - - With the *body* indented on the next line - - *Always* parenthesise the `else` part if it is a compound formula. -1. The `and` and `else` keywords *may* be placed on the same line as the closing parenthesis. -1. The `and` and `else` keywords *may* be "cuddled": `) else (` -1. *Always* qualify *calls* to predicates of the same class with `this`. -2. *Prefer* postfix casts `a.(Expr)` to prefix casts `(Expr)a`. - -### Examples - -```ql - argumentType.isImplicitlyConvertibleTo(parameterType) - or - argumentType instanceof NullType and - result.getParameter(i).isOut() and - parameterType instanceof SimpleType - or - reflectionOrDynamicArg(argumentType, parameterType) -``` - -```ql - this.getName() = "Finalize" and not exists(this.getAParameter()) -``` - -```ql - e1.getType() instanceof BoolType and ( - b1 = true - or - b1 = false - ) and ( - b2 = true - or - b2 = false - ) -``` - -```ql - if e1 instanceof BitwiseOrExpr or e1 instanceof LogicalOrExpr then ( - impliesSub(e1.(BinaryOperation).getAnOperand(), e2, b1, b2) and - b1 = false - ) else ( - e1.getType() instanceof BoolType and - e1 = e2 and - b1 = b2 and - (b1 = true or b1 = false) - ) -``` - -```ql - (x instanceof Exception implies x.isPublic()) and y instanceof Exception -``` - -```ql - x instanceof Exception implies (x.isPublic() and y instanceof Exception) -``` - -```ql - exists(Type arg | arg = this.getAChild() | arg instanceof TypeParameter) -``` - -```ql - exists(Type qualifierType | - this.hasNonExactQualifierType(qualifierType) | - result = getANonExactQualifierSubType(qualifierType) - ) -``` - -```ql - methods = count(Method m | t = m.getDeclaringType() and not ilc(m)) -``` - -```ql - if n = 0 then result = 1 else result = n * f(n - 1) -``` - -```ql - if n = 0 - then result = 1 - else result = n * f(n - 1) -``` - -```ql - if - n = 0 - then - result = 1 - else - result = n * f(n - 1) -``` - -```ql - if exists(this.getContainingType()) then ( - result = "A nested class" and - parentName = this.getContainingType().getFullyQualifiedName() - ) else ( - result = parentName + "." + this.getName() and - parentName = this.getNamespace().getFullyQualifiedName() - ) -``` - -## Glossary - -| Phrase | Meaning | -|-------------|----------| -| *[annotation](https://help.semmle.com/QL/QLLanguageSpecification.html#annotations)* | An additional specifier used to modify a declaration, such as `private`, `override`, `deprecated`, `pragma`, `bindingset`, or `cached`. | -| *body* | The text inside `{ }`, `( )`, or each section of an `if`-`then`-`else` or `from`-`where`-`select`. | -| *binary operator* | An operator with two operands, such as comparison operators, `and`, `or`, `implies`, or arithmetic operators. | -| *call* | A *formula* that invokes a predicate, e.g. `this.isStatic()` or `calls(a,b)`. | -| *[conjunct](https://help.semmle.com/QL/QLLanguageSpecification.html#conjunctions)* | A formula that is an operand to an `and`. | -| *declaration* | A class, module, predicate, field or newtype. | -| *[disjunct](https://help.semmle.com/QL/QLLanguageSpecification.html#disjunctions)* | A formula that is an operand to an `or`. | -| *[formula](https://help.semmle.com/QL/QLLanguageSpecification.html#formulas)* | A logical expression, such as `A = B`, a *call*, a *quantifier*, `and`, `or`, `not`, `in` or `instanceof`. | -| *should/should not/avoid/prefer* | Adhere to this rule wherever possible, where it makes sense. | -| *may/can* | This is a reasonable alternative, to be used with discretion. | -| *must/always/do not* | Always adhere to this rule. | -| *[quantifier/aggregation](https://help.semmle.com/QL/QLLanguageSpecification.html#aggregations)* | `exists`, `count`, `strictcount`, `any`, `forall`, `forex` and so on. | -| *variable* | A parameter to a predicate, a field, a from variable, or a variable introduced by a *quantifier* or *aggregation*. | +# QL Style Guide + +## Introduction + +This document describes how to format the QL code you contribute to this repository. It covers aspects such as layout, white-space, naming and documentation. Adhering to consistent standards makes code easier to read and maintain. Of course, these are only guidelines, and can be overridden as the need arises on a case-by-case basis. Where existing code deviates from these guidelines, prefer consistency with the surrounding code. + +Words in *italic* are defined in the [Glossary](#glossary). + +## Indentation +1. *Always* use 2 spaces for indentation. +1. *Always* indent: + - The *body* of a module, newtype, class or predicate + - The second and subsequent lines after you use a line break to split a long line + - The *body* of a `from`, `where` or `select` clause where it spans multiple lines + - The *body* of a *quantifier* that spans multiple lines + + +### Examples + +```ql +module Helpers { + /** ... */ + class X ... { + /** ... */ + int getNumberOfChildren () { + result = count(int child | + exists(this.getChild(child)) + ) + } + } +} +``` + +```ql +from Call c, string reason +where isDeprecated(c, reason) +select c, "This call to '$@' is deprecated because " + reason + ".", + c.getTarget(), c.getTarget().getName() +``` + +## Line breaks +1. Use UNIX line endings. +1. Lines *must not* exceed 100 characters. +1. Long lines *should* be split with a line break, and the following lines *must* be indented one level until the next "regular" line break. +1. There *should* be a single blank line: + - Between the file documentation and the first `import` + - Before each declaration, except for the first declaration in a *body* + - Before the `from`-`where`-`select` section in a query file +1. *Avoid* two or more adjacent blank lines. +1. There *must* be a new line after the *annotations* `cached`, `pragma`, `language` and `bindingset`. Other *annotations* do not have a new line. +1. There *should not* be additional blank lines within a predicate. +1. There *may* be a new line: + - Immediately after the `from`, `where` or `select` keywords in a query. + - Immediately after `if`, `then`, or `else` keywords. The `then` and `else` parts *should* be consistent. +1. *Avoid* other line breaks in declarations, other than to break long lines. +1. When operands of *binary operators* span two lines, the operator *should* be placed at the end of the first line. + +### Examples + +```ql +cached +private int getNumberOfParameters() { + ... +} +``` + +```ql +predicate methodStats(string qualifiedName, string name, + int numberOfParameters, int numberOfStatements, int numberOfExpressions, + int linesOfCode, int nestingDepth, int numberOfBranches) { + ... +} +``` + +```ql +from Method main +where main.getName() = "Main" +select main, "This is the program entry point." +``` + +```ql +from Method main +where + main.getName() = "Main" and + main.getNumberOfParameters() = 0 +select main, "Main method has no parameters." +``` + +```ql + if x.isPublic() + then result = "public" + else result = "private" +``` + +```ql + if x.isPublic() then + result = "public" + else + result = "private" +``` + +```ql + if + x.isPublic() + then + result = "public" + else + result = "private" +``` + +## Braces +1. Braces follow [Stroustrup](https://en.wikipedia.org/wiki/Indentation_style#Variant:_Stroustrup) style. The opening `{` *must* be placed at the end of the preceding line. +1. The closing `}` *must* be placed on its own line, indented to the outer level, or be on the same line as the opening `{`. +1. Braces of empty blocks *may* be placed on a single line, with a single space separating the braces. +1. Short predicates, not exceeding the maximum line width, *may* be placed on a single line, with a space following the opening brace and preceding the closing brace. + +### Examples + +```ql +class ThrowException extends ThrowExpr { + Foo() { + this.getTarget() instanceof ExceptionClass + } + + override string toString() { result = "Throw Exception" } +} +``` + +## Spaces +1. There *must* be a space or line break: + - Surrounding each `=` and `|` + - After each `,` +1. There *should* be a space or line break: + - Surrounding each *binary operator*, which *must* be balanced + - Surrounding `..` in a range + - Exceptions to this may be made to save space or to improve readability. +1. *Avoid* other spaces, for example: + - After a *quantifier/aggregation* keyword + - After the predicate name in a *call* + - Inside brackets used for *calls*, single-line quantifiers, and parenthesised formulas + - Surrounding a `.` + - Inside the opening or closing `[ ]` in a range expression + - Inside casts `a.(X)` +1. *Avoid* multiple spaces, except for indentation, and *avoid* additional indentation to align formulas, parameters or arguments. +1. *Do not* put whitespace on blank lines, or trailing on the end of a line. +1. *Do not* use tabs. + + +### Examples + +```ql +cached +private predicate foo(Expr e, Expr p) { + exists(int n | + n in [0 .. 1] | + e = p.getChild(n + 1) + ) +} +``` + +## Naming +1. Use [PascalCase](http://wiki.c2.com/?PascalCase) for: + - `class` names + - `module` names + - `newtype` names +1. Use [camelCase](https://en.wikipedia.org/wiki/Camel_case) for: + - Predicate names + - Variable names +1. Newtype predicate names *should* begin with `T`. +1. Predicates that have a result *should* be named `get...` +1. Predicates that can return multiple results *should* be named `getA...` or `getAn...` +1. Predicates that don't have a result or parameters *should* be named `is...` or `has...` +1. *Avoid* underscores in names. +1. *Avoid* short or single-letter names for classes, predicates and fields. +1. Short or single letter names for parameters and *quantifiers* *may* be used provided that they are sufficiently clear. +1. Use names as they are used in the target-language specification. +1. Use American English. + +### Examples + +```ql +/** ... */ +predicate calls(Callable caller, Callable callee) { + ... +} +``` + +```ql +/** ... */ +class Type extends ... { + /** ... */ + string getName() { ... } + + /** ... */ + predicate declares(Member m) { ... } + + /** ... */ + predicate isGeneric() { ... } + + /** ... */ + Type getTypeParameter(int n) { ... } + + /** ... */ + Type getATypeParameter() { ... } +} +``` + +## Documentation + +General requirements: + +1. Documentation *must* adhere to the [QLDoc specification](https://help.semmle.com/QL/QLDocSpecification.html). +1. Use `/** ... */` for documentation, even for single line comments. +1. For single-line documentation, the `/**` and `*/` are written on the same line as the comment. +1. For multi-line documentation, the `/**` and `*/` are written on separate lines. There is a `*` preceding each comment line, aligned on the first `*`. +1. Use full sentences, with capital letters and full stops. +1. Use American English. +1. Documentation comments *should* be appropriate for users of the code. +1. Documentation for maintainers of the code *must* use normal comments. + +Documentation for specific items: + +1. Public declarations *must* be documented. +1. Non-public declarations *should* be documented. +1. Declarations in query files *should* be documented. +1. Library files (`.qll` files) *should* be have a documentation comment at the top of the file. +1. Query files, except for tests, *must* have a QLDoc query documentation comment at the top of the file. +1. Predicates that do not have a result *should* be documented `/** Holds if ... */` +1. Predicates that have a result *should* be documented `/** Gets ... */` +1. All predicate parameters *should* be referred to in the predicate documentation. +1. Reference names, such as types and parameters, using backticks `` ` ``. +1. Give examples of code in the target language, enclosed in ```` ``` ```` or `` ` ``. +1. Classes *should* be documented in the singular, for example `/* An expression. */` +1. Where a class denotes a generic concept with subclasses, list those subclasses. +1. Declarations that are deprecated *should* be documented as `DEPRECATED: ...` +1. Declarations that are for internal use *should* be documented as `INTERNAL: Do not use`. + +### Examples + +```ql +/** Provides logic for determining constant expressions. */ +``` + +```ql +/** + * Holds if the qualifier of this call has type `qualifierType`. + * `isExactType` indicates whether the type is exact, that is, whether + * the qualifier is guaranteed not to be a subtype of `qualifierType`. + */ +``` +```ql +/** + * A delegate declaration, for example + * ``` + * delegate void Logger(string text); + * ``` + */ +class Delegate extends ... +``` + +```ql +/** + * An element that can be called. + * + * Either a method (`Method`), a constructor (`Constructor`), a destructor + * (`Destructor`), an operator (`Operator`), an accessor (`Accessor`), + * an anonymous function (`AnonymousFunctionExpr`), or a local function + * (`LocalFunction`). + */ +class Callable extends ... +``` + +```ql +/** DEPRECATED: Use `getAnExpr()` instead. */ +deprecated Expr getInitializer() +``` + +```ql +/** + * INTERNAL: Do not use. + */ +``` + +## Formulas +1. *Prefer* one *conjunct* per line. +1. Write the `and` at the end of the line. This also applies in `where` clauses. +1. *Prefer* to write the `or` keyword on its own line. +1. The `or` keyword *may* be written at the end of a line, or within a line, provided that it has no unparenthesised `and` operands. +1. Single-line formulas *may* be used in order to save space or add clarity, particularly in the *body* of a *quantifier/aggregation*. +1. *Always* use brackets to clarify the precedence of: + - `implies` + - `if`-`then`-`else` +1. Parenthesised formulas *can* be written: + - Within a single line. There *should not* be an additional space following the opening parenthesis or preceding the closing parenthesis. + - Spanning multiple lines. The opening parenthesis *should* be placed at the end of the preceding line, the body should be indented one level, and the closing bracket should be placed on a new line at the outer indentation. +1. *Quantifiers/aggregations* *can* be written: + - Within a single line. In this case, there is no space to the inside of the parentheses, or after the quantifier keyword. + - Across multiple lines. In this case, type declarations are on the same line as the quantifier, the `|` *may* be at the end of the line, or *may* be on its own line, and the body of the quantifier *must* be indented one level. The closing `)` is written on a new line, at the outer indentation. +1. `if`-`then`-`else` *can* be written: + - On a single line + - With the *body* after the `if`/`then`/`else` keyword + - With the *body* indented on the next line + - *Always* parenthesise the `else` part if it is a compound formula. +1. The `and` and `else` keywords *may* be placed on the same line as the closing parenthesis. +1. The `and` and `else` keywords *may* be "cuddled": `) else (` +1. *Always* qualify *calls* to predicates of the same class with `this`. +2. *Prefer* postfix casts `a.(Expr)` to prefix casts `(Expr)a`. + +### Examples + +```ql + argumentType.isImplicitlyConvertibleTo(parameterType) + or + argumentType instanceof NullType and + result.getParameter(i).isOut() and + parameterType instanceof SimpleType + or + reflectionOrDynamicArg(argumentType, parameterType) +``` + +```ql + this.getName() = "Finalize" and not exists(this.getAParameter()) +``` + +```ql + e1.getType() instanceof BoolType and ( + b1 = true + or + b1 = false + ) and ( + b2 = true + or + b2 = false + ) +``` + +```ql + if e1 instanceof BitwiseOrExpr or e1 instanceof LogicalOrExpr then ( + impliesSub(e1.(BinaryOperation).getAnOperand(), e2, b1, b2) and + b1 = false + ) else ( + e1.getType() instanceof BoolType and + e1 = e2 and + b1 = b2 and + (b1 = true or b1 = false) + ) +``` + +```ql + (x instanceof Exception implies x.isPublic()) and y instanceof Exception +``` + +```ql + x instanceof Exception implies (x.isPublic() and y instanceof Exception) +``` + +```ql + exists(Type arg | arg = this.getAChild() | arg instanceof TypeParameter) +``` + +```ql + exists(Type qualifierType | + this.hasNonExactQualifierType(qualifierType) | + result = getANonExactQualifierSubType(qualifierType) + ) +``` + +```ql + methods = count(Method m | t = m.getDeclaringType() and not ilc(m)) +``` + +```ql + if n = 0 then result = 1 else result = n * f(n - 1) +``` + +```ql + if n = 0 + then result = 1 + else result = n * f(n - 1) +``` + +```ql + if + n = 0 + then + result = 1 + else + result = n * f(n - 1) +``` + +```ql + if exists(this.getContainingType()) then ( + result = "A nested class" and + parentName = this.getContainingType().getFullyQualifiedName() + ) else ( + result = parentName + "." + this.getName() and + parentName = this.getNamespace().getFullyQualifiedName() + ) +``` + +## Glossary + +| Phrase | Meaning | +|-------------|----------| +| *[annotation](https://help.semmle.com/QL/QLLanguageSpecification.html#annotations)* | An additional specifier used to modify a declaration, such as `private`, `override`, `deprecated`, `pragma`, `bindingset`, or `cached`. | +| *body* | The text inside `{ }`, `( )`, or each section of an `if`-`then`-`else` or `from`-`where`-`select`. | +| *binary operator* | An operator with two operands, such as comparison operators, `and`, `or`, `implies`, or arithmetic operators. | +| *call* | A *formula* that invokes a predicate, e.g. `this.isStatic()` or `calls(a,b)`. | +| *[conjunct](https://help.semmle.com/QL/QLLanguageSpecification.html#conjunctions)* | A formula that is an operand to an `and`. | +| *declaration* | A class, module, predicate, field or newtype. | +| *[disjunct](https://help.semmle.com/QL/QLLanguageSpecification.html#disjunctions)* | A formula that is an operand to an `or`. | +| *[formula](https://help.semmle.com/QL/QLLanguageSpecification.html#formulas)* | A logical expression, such as `A = B`, a *call*, a *quantifier*, `and`, `or`, `not`, `in` or `instanceof`. | +| *should/should not/avoid/prefer* | Adhere to this rule wherever possible, where it makes sense. | +| *may/can* | This is a reasonable alternative, to be used with discretion. | +| *must/always/do not* | Always adhere to this rule. | +| *[quantifier/aggregation](https://help.semmle.com/QL/QLLanguageSpecification.html#aggregations)* | `exists`, `count`, `strictcount`, `any`, `forall`, `forex` and so on. | +| *variable* | A parameter to a predicate, a field, a from variable, or a variable introduced by a *quantifier* or *aggregation*. | diff --git a/java/ql/src/Frameworks/Spring/Architecture/Refactoring Opportunities/MissingParentBean.xml b/java/ql/src/Frameworks/Spring/Architecture/Refactoring Opportunities/MissingParentBean.xml index 9e7554e5254..6ed09e2920e 100644 --- a/java/ql/src/Frameworks/Spring/Architecture/Refactoring Opportunities/MissingParentBean.xml +++ b/java/ql/src/Frameworks/Spring/Architecture/Refactoring Opportunities/MissingParentBean.xml @@ -1,28 +1,28 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/java/ql/src/Frameworks/Spring/Architecture/Refactoring Opportunities/TooManyBeans.xml b/java/ql/src/Frameworks/Spring/Architecture/Refactoring Opportunities/TooManyBeans.xml index bd8b0ad7da1..0361ff562c0 100644 --- a/java/ql/src/Frameworks/Spring/Architecture/Refactoring Opportunities/TooManyBeans.xml +++ b/java/ql/src/Frameworks/Spring/Architecture/Refactoring Opportunities/TooManyBeans.xml @@ -1,8 +1,8 @@ - - - - - - - - + + + + + + + + diff --git a/java/ql/src/Frameworks/Spring/Architecture/Refactoring Opportunities/UnusedBean.xml b/java/ql/src/Frameworks/Spring/Architecture/Refactoring Opportunities/UnusedBean.xml index e40477b83c8..39ef911a3e6 100644 --- a/java/ql/src/Frameworks/Spring/Architecture/Refactoring Opportunities/UnusedBean.xml +++ b/java/ql/src/Frameworks/Spring/Architecture/Refactoring Opportunities/UnusedBean.xml @@ -1,6 +1,6 @@ - - - - - - + + + + + + diff --git a/java/ql/src/Frameworks/Spring/Architecture/Refactoring Opportunities/UselessPropertyOverride.xml b/java/ql/src/Frameworks/Spring/Architecture/Refactoring Opportunities/UselessPropertyOverride.xml index 9337a0f045d..9a8cceaf3dc 100644 --- a/java/ql/src/Frameworks/Spring/Architecture/Refactoring Opportunities/UselessPropertyOverride.xml +++ b/java/ql/src/Frameworks/Spring/Architecture/Refactoring Opportunities/UselessPropertyOverride.xml @@ -1,23 +1,23 @@ - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + diff --git a/java/ql/src/Frameworks/Spring/Violations of Best Practice/AvoidAutowiring.xml b/java/ql/src/Frameworks/Spring/Violations of Best Practice/AvoidAutowiring.xml index e7f707bdb65..2a10a0be651 100644 --- a/java/ql/src/Frameworks/Spring/Violations of Best Practice/AvoidAutowiring.xml +++ b/java/ql/src/Frameworks/Spring/Violations of Best Practice/AvoidAutowiring.xml @@ -1,13 +1,13 @@ - - - - - - - - + + + + + + + + \ No newline at end of file diff --git a/java/ql/src/Frameworks/Spring/Violations of Best Practice/DontUseConstructorArgIndex.xml b/java/ql/src/Frameworks/Spring/Violations of Best Practice/DontUseConstructorArgIndex.xml index 3e14a36dbce..92560855a7c 100644 --- a/java/ql/src/Frameworks/Spring/Violations of Best Practice/DontUseConstructorArgIndex.xml +++ b/java/ql/src/Frameworks/Spring/Violations of Best Practice/DontUseConstructorArgIndex.xml @@ -1,13 +1,13 @@ - - - - - - - - - - + + + + + + + + + + \ No newline at end of file diff --git a/java/ql/src/Frameworks/Spring/Violations of Best Practice/ImportsFirst.xml b/java/ql/src/Frameworks/Spring/Violations of Best Practice/ImportsFirst.xml index 739e375f28c..1199bb4de0e 100644 --- a/java/ql/src/Frameworks/Spring/Violations of Best Practice/ImportsFirst.xml +++ b/java/ql/src/Frameworks/Spring/Violations of Best Practice/ImportsFirst.xml @@ -1,26 +1,26 @@ - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/java/ql/src/Frameworks/Spring/Violations of Best Practice/NoBeanDescription.xml b/java/ql/src/Frameworks/Spring/Violations of Best Practice/NoBeanDescription.xml index 438d11b4449..7d6af942ca4 100644 --- a/java/ql/src/Frameworks/Spring/Violations of Best Practice/NoBeanDescription.xml +++ b/java/ql/src/Frameworks/Spring/Violations of Best Practice/NoBeanDescription.xml @@ -1,27 +1,27 @@ - - - - This file configures the various service beans. - - - - - - This bean defines base properties common to the service beans - - ... - - - - ... - - - - ... - + + + + This file configures the various service beans. + + + + + + This bean defines base properties common to the service beans + + ... + + + + ... + + + + ... + \ No newline at end of file diff --git a/java/ql/src/Frameworks/Spring/Violations of Best Practice/UseIdInsteadOfName.xml b/java/ql/src/Frameworks/Spring/Violations of Best Practice/UseIdInsteadOfName.xml index af75a2d37dd..f6121793781 100644 --- a/java/ql/src/Frameworks/Spring/Violations of Best Practice/UseIdInsteadOfName.xml +++ b/java/ql/src/Frameworks/Spring/Violations of Best Practice/UseIdInsteadOfName.xml @@ -1,16 +1,16 @@ - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/java/ql/src/Frameworks/Spring/Violations of Best Practice/UseLocalRef.xml b/java/ql/src/Frameworks/Spring/Violations of Best Practice/UseLocalRef.xml index b2dcdea2119..5b6256ee52c 100644 --- a/java/ql/src/Frameworks/Spring/Violations of Best Practice/UseLocalRef.xml +++ b/java/ql/src/Frameworks/Spring/Violations of Best Practice/UseLocalRef.xml @@ -1,17 +1,17 @@ - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/java/ql/src/Frameworks/Spring/Violations of Best Practice/UseSetterInjection.java b/java/ql/src/Frameworks/Spring/Violations of Best Practice/UseSetterInjection.java index d5cf0de1ea6..784b8ff9262 100644 --- a/java/ql/src/Frameworks/Spring/Violations of Best Practice/UseSetterInjection.java +++ b/java/ql/src/Frameworks/Spring/Violations of Best Practice/UseSetterInjection.java @@ -1,22 +1,22 @@ -// Class for bean 'chart1' -public class WrongChartMaker { - private AxisRenderer axisRenderer = new DefaultAxisRenderer(); - private TrendRenderer trendRenderer = new DefaultTrendRenderer(); - - public WrongChartMaker() {} - - // Each combination of the optional parameters must be represented by a constructor. - public WrongChartMaker(AxisRenderer customAxisRenderer) { - this.axisRenderer = customAxisRenderer; - } - - public WrongChartMaker(TrendRenderer customTrendRenderer) { - this.trendRenderer = customTrendRenderer; - } - - public WrongChartMaker(AxisRenderer customAxisRenderer, - TrendRenderer customTrendRenderer) { - this.axisRenderer = customAxisRenderer; - this.trendRenderer = customTrendRenderer; - } +// Class for bean 'chart1' +public class WrongChartMaker { + private AxisRenderer axisRenderer = new DefaultAxisRenderer(); + private TrendRenderer trendRenderer = new DefaultTrendRenderer(); + + public WrongChartMaker() {} + + // Each combination of the optional parameters must be represented by a constructor. + public WrongChartMaker(AxisRenderer customAxisRenderer) { + this.axisRenderer = customAxisRenderer; + } + + public WrongChartMaker(TrendRenderer customTrendRenderer) { + this.trendRenderer = customTrendRenderer; + } + + public WrongChartMaker(AxisRenderer customAxisRenderer, + TrendRenderer customTrendRenderer) { + this.axisRenderer = customAxisRenderer; + this.trendRenderer = customTrendRenderer; + } } \ No newline at end of file diff --git a/java/ql/src/Frameworks/Spring/Violations of Best Practice/UseShortcutForms.xml b/java/ql/src/Frameworks/Spring/Violations of Best Practice/UseShortcutForms.xml index 703e67ffe93..0f6afa629f6 100644 --- a/java/ql/src/Frameworks/Spring/Violations of Best Practice/UseShortcutForms.xml +++ b/java/ql/src/Frameworks/Spring/Violations of Best Practice/UseShortcutForms.xml @@ -1,38 +1,38 @@ - - - - main_service_registry - - - Top-level registry for services - - - - - - orderService - - com.foo.bar.OrderService - - - - billingService - - com.foo.bar.BillingService - - - - - - - - - - - - - - - - - + + + + main_service_registry + + + Top-level registry for services + + + + + + orderService + + com.foo.bar.OrderService + + + + billingService + + com.foo.bar.BillingService + + + + + + + + + + + + + + + + + diff --git a/java/ql/src/Frameworks/Spring/XML Configuration Errors/MissingSetters.java b/java/ql/src/Frameworks/Spring/XML Configuration Errors/MissingSetters.java index 6a310cfb758..9541f489ea6 100644 --- a/java/ql/src/Frameworks/Spring/XML Configuration Errors/MissingSetters.java +++ b/java/ql/src/Frameworks/Spring/XML Configuration Errors/MissingSetters.java @@ -1,9 +1,9 @@ -// bean class -public class ContentService { - private TransactionHelper helper; - - // This method does not match the property in the bean file. - public void setHelper(TransactionHelper helper) { - this.helper = helper; - } -} +// bean class +public class ContentService { + private TransactionHelper helper; + + // This method does not match the property in the bean file. + public void setHelper(TransactionHelper helper) { + this.helper = helper; + } +} diff --git a/java/ql/src/Frameworks/Spring/XML Configuration Errors/MissingSetters.xml b/java/ql/src/Frameworks/Spring/XML Configuration Errors/MissingSetters.xml index 9a4ac59115c..42a6b51ca02 100644 --- a/java/ql/src/Frameworks/Spring/XML Configuration Errors/MissingSetters.xml +++ b/java/ql/src/Frameworks/Spring/XML Configuration Errors/MissingSetters.xml @@ -1,7 +1,7 @@ - - - - - - + + + + + + diff --git a/java/ql/src/Likely Bugs/Arithmetic/BadAbsOfRandom.java b/java/ql/src/Likely Bugs/Arithmetic/BadAbsOfRandom.java index 96ab5134a36..ea5aa657ba0 100644 --- a/java/ql/src/Likely Bugs/Arithmetic/BadAbsOfRandom.java +++ b/java/ql/src/Likely Bugs/Arithmetic/BadAbsOfRandom.java @@ -1,17 +1,17 @@ -public static void main(String args[]) { - Random r = new Random(); - - // BAD: 'mayBeNegativeInt' is negative if - // 'nextInt()' returns 'Integer.MIN_VALUE'. - int mayBeNegativeInt = Math.abs(r.nextInt()); - - // GOOD: 'nonNegativeInt' is always a value between 0 (inclusive) - // and Integer.MAX_VALUE (exclusive). - int nonNegativeInt = r.nextInt(Integer.MAX_VALUE); - - // GOOD: When 'nextInt' returns a negative number increment the returned value. - int nextInt = r.nextInt(); - if(nextInt < 0) - nextInt++; - int nonNegativeInt = Math.abs(nextInt); -} +public static void main(String args[]) { + Random r = new Random(); + + // BAD: 'mayBeNegativeInt' is negative if + // 'nextInt()' returns 'Integer.MIN_VALUE'. + int mayBeNegativeInt = Math.abs(r.nextInt()); + + // GOOD: 'nonNegativeInt' is always a value between 0 (inclusive) + // and Integer.MAX_VALUE (exclusive). + int nonNegativeInt = r.nextInt(Integer.MAX_VALUE); + + // GOOD: When 'nextInt' returns a negative number increment the returned value. + int nextInt = r.nextInt(); + if(nextInt < 0) + nextInt++; + int nonNegativeInt = Math.abs(nextInt); +} diff --git a/java/ql/src/Likely Bugs/Arithmetic/RandomUsedOnce.java b/java/ql/src/Likely Bugs/Arithmetic/RandomUsedOnce.java index 63eaedb7717..bb1886a80fe 100644 --- a/java/ql/src/Likely Bugs/Arithmetic/RandomUsedOnce.java +++ b/java/ql/src/Likely Bugs/Arithmetic/RandomUsedOnce.java @@ -1,12 +1,12 @@ -public static void main(String args[]) { - // BAD: A new 'Random' object is created every time - // a pseudo-random integer is required. - int notReallyRandom = new Random().nextInt(); - int notReallyRandom2 = new Random().nextInt(); - - // GOOD: The same 'Random' object is used to generate - // two pseudo-random integers. - Random r = new Random(); - int random1 = r.nextInt(); - int random2 = r.nextInt(); +public static void main(String args[]) { + // BAD: A new 'Random' object is created every time + // a pseudo-random integer is required. + int notReallyRandom = new Random().nextInt(); + int notReallyRandom2 = new Random().nextInt(); + + // GOOD: The same 'Random' object is used to generate + // two pseudo-random integers. + Random r = new Random(); + int random1 = r.nextInt(); + int random2 = r.nextInt(); } \ No newline at end of file diff --git a/java/ql/src/Likely Bugs/Finalization/NullifiedSuperFinalize.java b/java/ql/src/Likely Bugs/Finalization/NullifiedSuperFinalize.java index ddd6d5c7f4f..811e3cdcac6 100644 --- a/java/ql/src/Likely Bugs/Finalization/NullifiedSuperFinalize.java +++ b/java/ql/src/Likely Bugs/Finalization/NullifiedSuperFinalize.java @@ -1,30 +1,30 @@ -class LocalCache { - private Collection localResources; - - //... - - protected void finalize() throws Throwable { - for (NativeResource r : localResources) { - r.dispose(); - } - }; -} - -class WrongCache extends LocalCache { - //... - @Override - protected void finalize() throws Throwable { - // BAD: Empty 'finalize', which does not call 'super.finalize'. - // Native resources in LocalCache are not disposed of. - } -} - -class RightCache extends LocalCache { - //... - @Override - protected void finalize() throws Throwable { - // GOOD: 'finalize' calls 'super.finalize'. - // Native resources in LocalCache are disposed of. - super.finalize(); - } +class LocalCache { + private Collection localResources; + + //... + + protected void finalize() throws Throwable { + for (NativeResource r : localResources) { + r.dispose(); + } + }; +} + +class WrongCache extends LocalCache { + //... + @Override + protected void finalize() throws Throwable { + // BAD: Empty 'finalize', which does not call 'super.finalize'. + // Native resources in LocalCache are not disposed of. + } +} + +class RightCache extends LocalCache { + //... + @Override + protected void finalize() throws Throwable { + // GOOD: 'finalize' calls 'super.finalize'. + // Native resources in LocalCache are disposed of. + super.finalize(); + } } \ No newline at end of file diff --git a/java/ql/src/Likely Bugs/Frameworks/JUnit/BadSuiteMethod.java b/java/ql/src/Likely Bugs/Frameworks/JUnit/BadSuiteMethod.java index 2cc578bfff7..9b17c5faf0c 100644 --- a/java/ql/src/Likely Bugs/Frameworks/JUnit/BadSuiteMethod.java +++ b/java/ql/src/Likely Bugs/Frameworks/JUnit/BadSuiteMethod.java @@ -1,21 +1,21 @@ -public class BadSuiteMethod extends TestCase { - // BAD: JUnit 3.8 does not detect the following method as a 'suite' method. - // The method should be public, static, and return 'junit.framework.Test' - // or one of its subtypes. - static Test suite() { - TestSuite suite = new TestSuite(); - suite.addTest(new MyTests("testEquals")); - suite.addTest(new MyTests("testNotEquals")); - return suite; - } -} - -public class CorrectSuiteMethod extends TestCase { - // GOOD: JUnit 3.8 correctly detects the following method as a 'suite' method. - public static Test suite() { - TestSuite suite = new TestSuite(); - suite.addTest(new MyTests("testEquals")); - suite.addTest(new MyTests("testNotEquals")); - return suite; - } +public class BadSuiteMethod extends TestCase { + // BAD: JUnit 3.8 does not detect the following method as a 'suite' method. + // The method should be public, static, and return 'junit.framework.Test' + // or one of its subtypes. + static Test suite() { + TestSuite suite = new TestSuite(); + suite.addTest(new MyTests("testEquals")); + suite.addTest(new MyTests("testNotEquals")); + return suite; + } +} + +public class CorrectSuiteMethod extends TestCase { + // GOOD: JUnit 3.8 correctly detects the following method as a 'suite' method. + public static Test suite() { + TestSuite suite = new TestSuite(); + suite.addTest(new MyTests("testEquals")); + suite.addTest(new MyTests("testNotEquals")); + return suite; + } } \ No newline at end of file diff --git a/java/ql/src/Likely Bugs/Frameworks/JUnit/TearDownNoSuper.java b/java/ql/src/Likely Bugs/Frameworks/JUnit/TearDownNoSuper.java index d05d4f16326..2a7ebea4d78 100644 --- a/java/ql/src/Likely Bugs/Frameworks/JUnit/TearDownNoSuper.java +++ b/java/ql/src/Likely Bugs/Frameworks/JUnit/TearDownNoSuper.java @@ -1,65 +1,65 @@ -// Abstract class that initializes then shuts down the -// framework after each set of tests -abstract class FrameworkTestCase extends TestCase { - @Override - protected void setUp() throws Exception { - super.setUp(); - Framework.init(); - } - - @Override - protected void tearDown() throws Exception { - super.tearDown(); - Framework.shutdown(); - } -} - -// The following classes extend 'FrameworkTestCase' to reuse the -// 'setUp' and 'tearDown' methods of the framework. - -public class TearDownNoSuper extends FrameworkTestCase { - @Override - protected void setUp() throws Exception { - super.setUp(); - } - - public void testFramework() { - //... - } - - public void testFramework2() { - //... - } - - @Override - protected void tearDown() throws Exception { - // BAD: Does not call 'super.tearDown'. May cause later tests to fail - // when they try to re-initialize an already initialized framework. - // Even if the framework allows re-initialization, it may maintain the - // internal state, which could affect the results of succeeding tests. - System.out.println("Tests complete"); - } -} - -public class TearDownSuper extends FrameworkTestCase { - @Override - protected void setUp() throws Exception { - super.setUp(); - } - - public void testFramework() { - //... - } - - public void testFramework2() { - //... - } - - @Override - protected void tearDown() throws Exception { - // GOOD: Correctly calls 'super.tearDown' to shut down the - // framework. - System.out.println("Tests complete"); - super.tearDown(); - } +// Abstract class that initializes then shuts down the +// framework after each set of tests +abstract class FrameworkTestCase extends TestCase { + @Override + protected void setUp() throws Exception { + super.setUp(); + Framework.init(); + } + + @Override + protected void tearDown() throws Exception { + super.tearDown(); + Framework.shutdown(); + } +} + +// The following classes extend 'FrameworkTestCase' to reuse the +// 'setUp' and 'tearDown' methods of the framework. + +public class TearDownNoSuper extends FrameworkTestCase { + @Override + protected void setUp() throws Exception { + super.setUp(); + } + + public void testFramework() { + //... + } + + public void testFramework2() { + //... + } + + @Override + protected void tearDown() throws Exception { + // BAD: Does not call 'super.tearDown'. May cause later tests to fail + // when they try to re-initialize an already initialized framework. + // Even if the framework allows re-initialization, it may maintain the + // internal state, which could affect the results of succeeding tests. + System.out.println("Tests complete"); + } +} + +public class TearDownSuper extends FrameworkTestCase { + @Override + protected void setUp() throws Exception { + super.setUp(); + } + + public void testFramework() { + //... + } + + public void testFramework2() { + //... + } + + @Override + protected void tearDown() throws Exception { + // GOOD: Correctly calls 'super.tearDown' to shut down the + // framework. + System.out.println("Tests complete"); + super.tearDown(); + } } \ No newline at end of file diff --git a/java/ql/src/Likely Bugs/Frameworks/JUnit/TestCaseNoTests.java b/java/ql/src/Likely Bugs/Frameworks/JUnit/TestCaseNoTests.java index d9ea2cb651d..9c6b6f403de 100644 --- a/java/ql/src/Likely Bugs/Frameworks/JUnit/TestCaseNoTests.java +++ b/java/ql/src/Likely Bugs/Frameworks/JUnit/TestCaseNoTests.java @@ -1,28 +1,28 @@ -// BAD: This test case class does not have any valid JUnit 3.8 test methods. -public class TestCaseNoTests38 extends TestCase { - // This is not a test case because it does not start with 'test'. - public void simpleTest() { - //... - } - - // This is not a test case because it takes two parameters. - public void testNotEquals(int i, int j) { - assertEquals(i != j, true); - } - - // This is recognized as a test, but causes JUnit to fail - // when run because it is not public. - void testEquals() { - //... - } -} - -// GOOD: This test case class correctly declares test methods. -public class MyTests extends TestCase { - public void testEquals() { - assertEquals(1, 1); - } - public void testNotEquals() { - assertFalse(1 == 2); - } +// BAD: This test case class does not have any valid JUnit 3.8 test methods. +public class TestCaseNoTests38 extends TestCase { + // This is not a test case because it does not start with 'test'. + public void simpleTest() { + //... + } + + // This is not a test case because it takes two parameters. + public void testNotEquals(int i, int j) { + assertEquals(i != j, true); + } + + // This is recognized as a test, but causes JUnit to fail + // when run because it is not public. + void testEquals() { + //... + } +} + +// GOOD: This test case class correctly declares test methods. +public class MyTests extends TestCase { + public void testEquals() { + assertEquals(1, 1); + } + public void testNotEquals() { + assertFalse(1 == 2); + } } \ No newline at end of file diff --git a/java/ql/src/Likely Bugs/I18N/MissingLocaleArgument.java b/java/ql/src/Likely Bugs/I18N/MissingLocaleArgument.java index 741bc708905..382c80c209a 100644 --- a/java/ql/src/Likely Bugs/I18N/MissingLocaleArgument.java +++ b/java/ql/src/Likely Bugs/I18N/MissingLocaleArgument.java @@ -1,14 +1,14 @@ -public static void main(String args[]) { - String phrase = "I miss my home in Mississippi."; - - // AVOID: Calling 'toLowerCase()' or 'toUpperCase()' - // produces different results depending on what the default locale is. - System.out.println(phrase.toUpperCase()); - System.out.println(phrase.toLowerCase()); - - // GOOD: Explicitly setting the locale when calling 'toLowerCase()' or - // 'toUpperCase()' ensures that the resulting string is - // English, regardless of the default locale. - System.out.println(phrase.toLowerCase(Locale.ENGLISH)); - System.out.println(phrase.toUpperCase(Locale.ENGLISH)); +public static void main(String args[]) { + String phrase = "I miss my home in Mississippi."; + + // AVOID: Calling 'toLowerCase()' or 'toUpperCase()' + // produces different results depending on what the default locale is. + System.out.println(phrase.toUpperCase()); + System.out.println(phrase.toLowerCase()); + + // GOOD: Explicitly setting the locale when calling 'toLowerCase()' or + // 'toUpperCase()' ensures that the resulting string is + // English, regardless of the default locale. + System.out.println(phrase.toLowerCase(Locale.ENGLISH)); + System.out.println(phrase.toUpperCase(Locale.ENGLISH)); } \ No newline at end of file diff --git a/java/ql/src/Likely Bugs/Serialization/IncorrectSerialVersionUID.java b/java/ql/src/Likely Bugs/Serialization/IncorrectSerialVersionUID.java index 91ee7d116cd..eaf9e459780 100644 --- a/java/ql/src/Likely Bugs/Serialization/IncorrectSerialVersionUID.java +++ b/java/ql/src/Likely Bugs/Serialization/IncorrectSerialVersionUID.java @@ -1,11 +1,11 @@ -class WrongNote implements Serializable { - // BAD: serialVersionUID must be static, final, and 'long' - private static final int serialVersionUID = 1; - - //... -} - -class Note implements Serializable { - // GOOD: serialVersionUID is of the correct type - private static final long serialVersionUID = 1L; +class WrongNote implements Serializable { + // BAD: serialVersionUID must be static, final, and 'long' + private static final int serialVersionUID = 1; + + //... +} + +class Note implements Serializable { + // GOOD: serialVersionUID is of the correct type + private static final long serialVersionUID = 1L; } \ No newline at end of file diff --git a/java/ql/src/Likely Bugs/Serialization/IncorrectSerializableMethods.java b/java/ql/src/Likely Bugs/Serialization/IncorrectSerializableMethods.java index 33599f15e25..57fd8a64cc2 100644 --- a/java/ql/src/Likely Bugs/Serialization/IncorrectSerializableMethods.java +++ b/java/ql/src/Likely Bugs/Serialization/IncorrectSerializableMethods.java @@ -1,25 +1,25 @@ -class WrongNetRequest implements Serializable { - // BAD: Does not match the exact signature required for a custom - // deserialization protocol. Will not be called during deserialization. - void readObject(ObjectInputStream in) { - //... - } - - // BAD: Does not match the exact signature required for a custom - // serialization protocol. Will not be called during serialization. - protected void writeObject(ObjectOutputStream out) { - //... - } -} - -class NetRequest implements Serializable { - // GOOD: Signature for a custom deserialization implementation. - private void readObject(ObjectInputStream in) { - //... - } - - // GOOD: Signature for a custom serialization implementation. - private void writeObject(ObjectOutputStream out) { - //... - } +class WrongNetRequest implements Serializable { + // BAD: Does not match the exact signature required for a custom + // deserialization protocol. Will not be called during deserialization. + void readObject(ObjectInputStream in) { + //... + } + + // BAD: Does not match the exact signature required for a custom + // serialization protocol. Will not be called during serialization. + protected void writeObject(ObjectOutputStream out) { + //... + } +} + +class NetRequest implements Serializable { + // GOOD: Signature for a custom deserialization implementation. + private void readObject(ObjectInputStream in) { + //... + } + + // GOOD: Signature for a custom serialization implementation. + private void writeObject(ObjectOutputStream out) { + //... + } } \ No newline at end of file diff --git a/java/ql/src/Likely Bugs/Serialization/MissingVoidConstructorOnExternalizable.java b/java/ql/src/Likely Bugs/Serialization/MissingVoidConstructorOnExternalizable.java index fc297ce1788..24c8d634d21 100644 --- a/java/ql/src/Likely Bugs/Serialization/MissingVoidConstructorOnExternalizable.java +++ b/java/ql/src/Likely Bugs/Serialization/MissingVoidConstructorOnExternalizable.java @@ -1,37 +1,37 @@ -class WrongMemo implements Externalizable { - private String memo; - - // BAD: No public no-argument constructor is defined. Deserializing this object - // causes an 'InvalidClassException'. - - public WrongMemo(String memo) { - this.memo = memo; - } - - public void writeExternal(ObjectOutput arg0) throws IOException { - //... - } - public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException { - //... - } -} - -class Memo implements Externalizable { - private String memo; - - // GOOD: Declare a public no-argument constructor, which is used by the - // serialization framework when the object is deserialized. - public Memo() { - } - - public Memo(String memo) { - this.memo = memo; - } - - public void writeExternal(ObjectOutput out) throws IOException { - //... - } - public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException { - //... - } +class WrongMemo implements Externalizable { + private String memo; + + // BAD: No public no-argument constructor is defined. Deserializing this object + // causes an 'InvalidClassException'. + + public WrongMemo(String memo) { + this.memo = memo; + } + + public void writeExternal(ObjectOutput arg0) throws IOException { + //... + } + public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException { + //... + } +} + +class Memo implements Externalizable { + private String memo; + + // GOOD: Declare a public no-argument constructor, which is used by the + // serialization framework when the object is deserialized. + public Memo() { + } + + public Memo(String memo) { + this.memo = memo; + } + + public void writeExternal(ObjectOutput out) throws IOException { + //... + } + public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException { + //... + } } \ No newline at end of file diff --git a/java/ql/src/Likely Bugs/Serialization/MissingVoidConstructorsOnSerializable.java b/java/ql/src/Likely Bugs/Serialization/MissingVoidConstructorsOnSerializable.java index d9161f38710..fea0087ba4f 100644 --- a/java/ql/src/Likely Bugs/Serialization/MissingVoidConstructorsOnSerializable.java +++ b/java/ql/src/Likely Bugs/Serialization/MissingVoidConstructorsOnSerializable.java @@ -1,42 +1,42 @@ -class WrongItem { - private String name; - - // BAD: This class does not have a no-argument constructor, and throws an - // 'InvalidClassException' at runtime. - - public WrongItem(String name) { - this.name = name; - } -} - -class WrongSubItem extends WrongItem implements Serializable { - public WrongSubItem() { - super(null); - } - - public WrongSubItem(String name) { - super(name); - } -} - -class Item { - private String name; - - // GOOD: This class declares a no-argument constructor, which allows serializable - // subclasses to be deserialized without error. - public Item() {} - - public Item(String name) { - this.name = name; - } -} - -class SubItem extends Item implements Serializable { - public SubItem() { - super(null); - } - - public SubItem(String name) { - super(name); - } +class WrongItem { + private String name; + + // BAD: This class does not have a no-argument constructor, and throws an + // 'InvalidClassException' at runtime. + + public WrongItem(String name) { + this.name = name; + } +} + +class WrongSubItem extends WrongItem implements Serializable { + public WrongSubItem() { + super(null); + } + + public WrongSubItem(String name) { + super(name); + } +} + +class Item { + private String name; + + // GOOD: This class declares a no-argument constructor, which allows serializable + // subclasses to be deserialized without error. + public Item() {} + + public Item(String name) { + this.name = name; + } +} + +class SubItem extends Item implements Serializable { + public SubItem() { + super(null); + } + + public SubItem(String name) { + super(name); + } } \ No newline at end of file diff --git a/java/ql/src/Likely Bugs/Serialization/NonSerializableComparator.java b/java/ql/src/Likely Bugs/Serialization/NonSerializableComparator.java index 1617440e762..d2d758bcebc 100644 --- a/java/ql/src/Likely Bugs/Serialization/NonSerializableComparator.java +++ b/java/ql/src/Likely Bugs/Serialization/NonSerializableComparator.java @@ -1,16 +1,16 @@ -// BAD: This is not serializable, and throws a 'java.io.NotSerializableException' -// when used in a serializable sorted collection. -class WrongComparator implements Comparator { - public int compare(String o1, String o2) { - return o1.compareTo(o2); - } -} - -// GOOD: This is serializable, and can be used in collections that are meant to be serialized. -class StringComparator implements Comparator, Serializable { - private static final long serialVersionUID = -5972458403679726498L; - - public int compare(String arg0, String arg1) { - return arg0.compareTo(arg1); - } +// BAD: This is not serializable, and throws a 'java.io.NotSerializableException' +// when used in a serializable sorted collection. +class WrongComparator implements Comparator { + public int compare(String o1, String o2) { + return o1.compareTo(o2); + } +} + +// GOOD: This is serializable, and can be used in collections that are meant to be serialized. +class StringComparator implements Comparator, Serializable { + private static final long serialVersionUID = -5972458403679726498L; + + public int compare(String arg0, String arg1) { + return arg0.compareTo(arg1); + } } \ No newline at end of file diff --git a/java/ql/src/Likely Bugs/Serialization/NonSerializableField.java b/java/ql/src/Likely Bugs/Serialization/NonSerializableField.java index 999e0dc207a..10821b8d785 100644 --- a/java/ql/src/Likely Bugs/Serialization/NonSerializableField.java +++ b/java/ql/src/Likely Bugs/Serialization/NonSerializableField.java @@ -1,27 +1,27 @@ -class DerivedFactors { // Class that contains derived values computed from entries in a - private Number efficiency; // performance record - private Number costPerItem; - private Number profitPerItem; - ... -} - -class WrongPerformanceRecord implements Serializable { - private String unitId; - private Number dailyThroughput; - private Number dailyCost; - private DerivedFactors factors; // BAD: 'DerivedFactors' is not serializable - // but is in a serializable class. This - // causes a 'java.io.NotSerializableException' - // when 'WrongPerformanceRecord' is serialized. - ... -} - -class PerformanceRecord implements Serializable { - private String unitId; - private Number dailyThroughput; - private Number dailyCost; - transient private DerivedFactors factors; // GOOD: 'DerivedFactors' is declared - // 'transient' so it does not contribute to the - // serializable state of 'PerformanceRecord'. - ... -} +class DerivedFactors { // Class that contains derived values computed from entries in a + private Number efficiency; // performance record + private Number costPerItem; + private Number profitPerItem; + ... +} + +class WrongPerformanceRecord implements Serializable { + private String unitId; + private Number dailyThroughput; + private Number dailyCost; + private DerivedFactors factors; // BAD: 'DerivedFactors' is not serializable + // but is in a serializable class. This + // causes a 'java.io.NotSerializableException' + // when 'WrongPerformanceRecord' is serialized. + ... +} + +class PerformanceRecord implements Serializable { + private String unitId; + private Number dailyThroughput; + private Number dailyCost; + transient private DerivedFactors factors; // GOOD: 'DerivedFactors' is declared + // 'transient' so it does not contribute to the + // serializable state of 'PerformanceRecord'. + ... +} diff --git a/java/ql/src/Likely Bugs/Serialization/NonSerializableFieldTooGeneral.java b/java/ql/src/Likely Bugs/Serialization/NonSerializableFieldTooGeneral.java index b2d51cf73e8..1a4a5cc04ca 100644 --- a/java/ql/src/Likely Bugs/Serialization/NonSerializableFieldTooGeneral.java +++ b/java/ql/src/Likely Bugs/Serialization/NonSerializableFieldTooGeneral.java @@ -1,29 +1,29 @@ -class WrongPair implements Serializable{ - private final L left; // BAD - private final R right; // BAD: L and R are not guaranteed to be serializable - - public WrongPair(L left, R right){ ... } - - ... -} - -class Pair implements Serializable{ - private final L left; // GOOD: L and R must implement Serializable - private final R right; - - public Pair(L left, R right){ ... } - - ... -} - -class WrongEvent implements Serializable{ - private Object eventData; // BAD: Type is too general. - - public WrongEvent(Object eventData){ ... } -} - -class Event implements Serializable{ - private Serializable eventData; // GOOD: Force the user to supply only serializable data - - public Event(Serializable eventData){ ... } -} +class WrongPair implements Serializable{ + private final L left; // BAD + private final R right; // BAD: L and R are not guaranteed to be serializable + + public WrongPair(L left, R right){ ... } + + ... +} + +class Pair implements Serializable{ + private final L left; // GOOD: L and R must implement Serializable + private final R right; + + public Pair(L left, R right){ ... } + + ... +} + +class WrongEvent implements Serializable{ + private Object eventData; // BAD: Type is too general. + + public WrongEvent(Object eventData){ ... } +} + +class Event implements Serializable{ + private Serializable eventData; // GOOD: Force the user to supply only serializable data + + public Event(Serializable eventData){ ... } +} diff --git a/java/ql/src/Likely Bugs/Serialization/NonSerializableInnerClass.java b/java/ql/src/Likely Bugs/Serialization/NonSerializableInnerClass.java index d2d44110812..e9a66f56a4a 100644 --- a/java/ql/src/Likely Bugs/Serialization/NonSerializableInnerClass.java +++ b/java/ql/src/Likely Bugs/Serialization/NonSerializableInnerClass.java @@ -1,33 +1,33 @@ -class NonSerializableServer { - - // BAD: The following class is serializable, but the enclosing class - // 'NonSerializableServer' is not. Serializing an instance of 'WrongSession' - // causes a 'java.io.NotSerializableException'. - class WrongSession implements Serializable { - private static final long serialVersionUID = 8970783971992397218L; - private int id; - private String user; - - WrongSession(int id, String user) { /*...*/ } - } - - public WrongSession getNewSession(String user) { - return new WrongSession(newId(), user); - } -} - -class Server { - - // GOOD: The following class can be correctly serialized because it is static. - static class Session implements Serializable { - private static final long serialVersionUID = 1065454318648105638L; - private int id; - private String user; - - Session(int id, String user) { /*...*/ } - } - - public Session getNewSession(String user) { - return new Session(newId(), user); - } +class NonSerializableServer { + + // BAD: The following class is serializable, but the enclosing class + // 'NonSerializableServer' is not. Serializing an instance of 'WrongSession' + // causes a 'java.io.NotSerializableException'. + class WrongSession implements Serializable { + private static final long serialVersionUID = 8970783971992397218L; + private int id; + private String user; + + WrongSession(int id, String user) { /*...*/ } + } + + public WrongSession getNewSession(String user) { + return new WrongSession(newId(), user); + } +} + +class Server { + + // GOOD: The following class can be correctly serialized because it is static. + static class Session implements Serializable { + private static final long serialVersionUID = 1065454318648105638L; + private int id; + private String user; + + Session(int id, String user) { /*...*/ } + } + + public Session getNewSession(String user) { + return new Session(newId(), user); + } } \ No newline at end of file diff --git a/java/ql/src/Likely Bugs/Serialization/ReadResolveObject.java b/java/ql/src/Likely Bugs/Serialization/ReadResolveObject.java index cba64daf49c..25057e1ecf2 100644 --- a/java/ql/src/Likely Bugs/Serialization/ReadResolveObject.java +++ b/java/ql/src/Likely Bugs/Serialization/ReadResolveObject.java @@ -1,40 +1,40 @@ -class FalseSingleton implements Serializable { - private static final long serialVersionUID = -7480651116825504381L; - private static FalseSingleton instance; - - private FalseSingleton() {} - - public static FalseSingleton getInstance() { - if (instance == null) { - instance = new FalseSingleton(); - } - return instance; - } - - // BAD: Signature of 'readResolve' does not match the exact signature that is expected - // (that is, it does not return 'java.lang.Object'). - public FalseSingleton readResolve() throws ObjectStreamException { - return FalseSingleton.getInstance(); - } -} - -class Singleton implements Serializable { - private static final long serialVersionUID = -7480651116825504381L; - private static Singleton instance; - - private Singleton() {} - - public static Singleton getInstance() { - if (instance == null) { - instance = new Singleton(); - } - return instance; - } - - // GOOD: Signature of 'readResolve' matches the exact signature that is expected. - // It replaces the singleton that is read from a stream with an instance of 'Singleton', - // instead of creating a new singleton. - private Object readResolve() throws ObjectStreamException { - return Singleton.getInstance(); - } +class FalseSingleton implements Serializable { + private static final long serialVersionUID = -7480651116825504381L; + private static FalseSingleton instance; + + private FalseSingleton() {} + + public static FalseSingleton getInstance() { + if (instance == null) { + instance = new FalseSingleton(); + } + return instance; + } + + // BAD: Signature of 'readResolve' does not match the exact signature that is expected + // (that is, it does not return 'java.lang.Object'). + public FalseSingleton readResolve() throws ObjectStreamException { + return FalseSingleton.getInstance(); + } +} + +class Singleton implements Serializable { + private static final long serialVersionUID = -7480651116825504381L; + private static Singleton instance; + + private Singleton() {} + + public static Singleton getInstance() { + if (instance == null) { + instance = new Singleton(); + } + return instance; + } + + // GOOD: Signature of 'readResolve' matches the exact signature that is expected. + // It replaces the singleton that is read from a stream with an instance of 'Singleton', + // instead of creating a new singleton. + private Object readResolve() throws ObjectStreamException { + return Singleton.getInstance(); + } } \ No newline at end of file diff --git a/java/ql/src/Likely Bugs/Serialization/TransientNotSerializable.java b/java/ql/src/Likely Bugs/Serialization/TransientNotSerializable.java index 783ed4f4181..3025f29bd5e 100644 --- a/java/ql/src/Likely Bugs/Serialization/TransientNotSerializable.java +++ b/java/ql/src/Likely Bugs/Serialization/TransientNotSerializable.java @@ -1,12 +1,12 @@ -class State { - // The 'transient' modifier has no effect here because - // the 'State' class does not implement 'Serializable'. - private transient int[] stateData; -} - -class PersistentState implements Serializable { - private int[] stateData; - // The 'transient' modifier indicates that this field is not part of - // the persistent state and should therefore not be serialized. - private transient int[] cachedComputedData; +class State { + // The 'transient' modifier has no effect here because + // the 'State' class does not implement 'Serializable'. + private transient int[] stateData; +} + +class PersistentState implements Serializable { + private int[] stateData; + // The 'transient' modifier indicates that this field is not part of + // the persistent state and should therefore not be serialized. + private transient int[] cachedComputedData; } \ No newline at end of file diff --git a/java/ql/src/Violations of Best Practice/Dead Code/FinalizerNullsFields.java b/java/ql/src/Violations of Best Practice/Dead Code/FinalizerNullsFields.java index d55d76f835f..7213d78104b 100644 --- a/java/ql/src/Violations of Best Practice/Dead Code/FinalizerNullsFields.java +++ b/java/ql/src/Violations of Best Practice/Dead Code/FinalizerNullsFields.java @@ -1,14 +1,14 @@ -class FinalizedClass { - Object o = new Object(); - String s = "abcdefg"; - Integer i = Integer.valueOf(2); - - @Override - protected void finalize() throws Throwable { - super.finalize(); - //No need to nullify fields - this.o = null; - this.s = null; - this.i = null; - } +class FinalizedClass { + Object o = new Object(); + String s = "abcdefg"; + Integer i = Integer.valueOf(2); + + @Override + protected void finalize() throws Throwable { + super.finalize(); + //No need to nullify fields + this.o = null; + this.s = null; + this.i = null; + } } \ No newline at end of file diff --git a/java/ql/src/Violations of Best Practice/Exception Handling/DroppedExceptions-comment.java b/java/ql/src/Violations of Best Practice/Exception Handling/DroppedExceptions-comment.java index a71288df47d..75f45e61ffc 100644 --- a/java/ql/src/Violations of Best Practice/Exception Handling/DroppedExceptions-comment.java +++ b/java/ql/src/Violations of Best Practice/Exception Handling/DroppedExceptions-comment.java @@ -1,9 +1,9 @@ -synchronized void waitIfAutoSyncScheduled() { - try { - while (isAutoSyncScheduled) { - this.wait(1000); - } - } catch (InterruptedException e) { - // Expected exception. The file cannot be synchronized yet. - } +synchronized void waitIfAutoSyncScheduled() { + try { + while (isAutoSyncScheduled) { + this.wait(1000); + } + } catch (InterruptedException e) { + // Expected exception. The file cannot be synchronized yet. + } } \ No newline at end of file diff --git a/java/ql/src/Violations of Best Practice/Exception Handling/DroppedExceptions-good.java b/java/ql/src/Violations of Best Practice/Exception Handling/DroppedExceptions-good.java index 0085f15c2db..4aeb0f464c9 100644 --- a/java/ql/src/Violations of Best Practice/Exception Handling/DroppedExceptions-good.java +++ b/java/ql/src/Violations of Best Practice/Exception Handling/DroppedExceptions-good.java @@ -1,10 +1,10 @@ -// Exception is passed to 'ignore' method with a comment -synchronized void waitIfAutoSyncScheduled() { - try { - while (isAutoSyncScheduled) { - this.wait(1000); - } - } catch (InterruptedException e) { - Exceptions.ignore(e, "Expected exception. The file cannot be synchronized yet."); - } +// Exception is passed to 'ignore' method with a comment +synchronized void waitIfAutoSyncScheduled() { + try { + while (isAutoSyncScheduled) { + this.wait(1000); + } + } catch (InterruptedException e) { + Exceptions.ignore(e, "Expected exception. The file cannot be synchronized yet."); + } } \ No newline at end of file diff --git a/java/ql/src/Violations of Best Practice/Exception Handling/DroppedExceptions-ignore.java b/java/ql/src/Violations of Best Practice/Exception Handling/DroppedExceptions-ignore.java index 6985620e530..266105c45c2 100644 --- a/java/ql/src/Violations of Best Practice/Exception Handling/DroppedExceptions-ignore.java +++ b/java/ql/src/Violations of Best Practice/Exception Handling/DroppedExceptions-ignore.java @@ -1,5 +1,5 @@ -// 'ignore' method. This method does nothing, but can be called -// to document the reason why the exception can be ignored. -public static void ignore(Throwable e, String message) { - +// 'ignore' method. This method does nothing, but can be called +// to document the reason why the exception can be ignored. +public static void ignore(Throwable e, String message) { + } \ No newline at end of file diff --git a/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToRunFinalizersOnExit.java b/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToRunFinalizersOnExit.java index 06655aa8c1f..0a7ec51c920 100644 --- a/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToRunFinalizersOnExit.java +++ b/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToRunFinalizersOnExit.java @@ -1,8 +1,8 @@ -void main() { - // ... - // BAD: Call to 'runFinalizersOnExit' forces execution of all finalizers on termination of - // the runtime, which can cause live objects to transition to an invalid state. - // Avoid using this method (and finalizers in general). - System.runFinalizersOnExit(true); - // ... +void main() { + // ... + // BAD: Call to 'runFinalizersOnExit' forces execution of all finalizers on termination of + // the runtime, which can cause live objects to transition to an invalid state. + // Avoid using this method (and finalizers in general). + System.runFinalizersOnExit(true); + // ... } \ No newline at end of file diff --git a/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToStringToString.java b/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToStringToString.java index 2fb4df11283..476ac9d49f5 100644 --- a/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToStringToString.java +++ b/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToStringToString.java @@ -1,9 +1,9 @@ -public static void main(String args[]) { - String name = "John Doe"; - - // BAD: Unnecessary call to 'toString' on 'name' - System.out.println("Hi, my name is " + name.toString()); - - // GOOD: No call to 'toString' on 'name' - System.out.println("Hi, my name is " + name); +public static void main(String args[]) { + String name = "John Doe"; + + // BAD: Unnecessary call to 'toString' on 'name' + System.out.println("Hi, my name is " + name.toString()); + + // GOOD: No call to 'toString' on 'name' + System.out.println("Hi, my name is " + name); } \ No newline at end of file diff --git a/java/ql/src/Violations of Best Practice/Undesirable Calls/DefaultToString.java b/java/ql/src/Violations of Best Practice/Undesirable Calls/DefaultToString.java index 2cff927b157..7363006363c 100644 --- a/java/ql/src/Violations of Best Practice/Undesirable Calls/DefaultToString.java +++ b/java/ql/src/Violations of Best Practice/Undesirable Calls/DefaultToString.java @@ -1,21 +1,21 @@ -// This class does not have a 'toString' method, so 'java.lang.Object.toString' -// is used when the class is converted to a string. -class WrongPerson { - private String name; - private Date birthDate; - - public WrongPerson(String name, Date birthDate) { - this.name =name; - this.birthDate = birthDate; - } -} - -public static void main(String args[]) throws Exception { - DateFormat dateFormatter = new SimpleDateFormat("yyyy-MM-dd"); - WrongPerson wp = new WrongPerson("Robert Van Winkle", dateFormatter.parse("1967-10-31")); - - // BAD: The following statement implicitly calls 'Object.toString', - // which returns something similar to: - // WrongPerson@4383f74d - System.out.println(wp); +// This class does not have a 'toString' method, so 'java.lang.Object.toString' +// is used when the class is converted to a string. +class WrongPerson { + private String name; + private Date birthDate; + + public WrongPerson(String name, Date birthDate) { + this.name =name; + this.birthDate = birthDate; + } +} + +public static void main(String args[]) throws Exception { + DateFormat dateFormatter = new SimpleDateFormat("yyyy-MM-dd"); + WrongPerson wp = new WrongPerson("Robert Van Winkle", dateFormatter.parse("1967-10-31")); + + // BAD: The following statement implicitly calls 'Object.toString', + // which returns something similar to: + // WrongPerson@4383f74d + System.out.println(wp); } \ No newline at end of file diff --git a/java/ql/src/Violations of Best Practice/Undesirable Calls/GarbageCollection.java b/java/ql/src/Violations of Best Practice/Undesirable Calls/GarbageCollection.java index 47c881dbf20..16d0f46fe09 100644 --- a/java/ql/src/Violations of Best Practice/Undesirable Calls/GarbageCollection.java +++ b/java/ql/src/Violations of Best Practice/Undesirable Calls/GarbageCollection.java @@ -1,15 +1,15 @@ -class RequestHandler extends Thread { - private boolean isRunning; - private Connection conn = new Connection(); - - public void run() { - while (isRunning) { - Request req = conn.getRequest(); - // Process the request ... - - System.gc(); // This call may cause a garbage collection after each request. - // This will likely reduce the throughput of the RequestHandler - // because the JVM spends time on unnecessary garbage collection passes. - } - } +class RequestHandler extends Thread { + private boolean isRunning; + private Connection conn = new Connection(); + + public void run() { + while (isRunning) { + Request req = conn.getRequest(); + // Process the request ... + + System.gc(); // This call may cause a garbage collection after each request. + // This will likely reduce the throughput of the RequestHandler + // because the JVM spends time on unnecessary garbage collection passes. + } + } } \ No newline at end of file diff --git a/java/ql/src/Violations of Best Practice/Undesirable Calls/PrintLnArray.java b/java/ql/src/Violations of Best Practice/Undesirable Calls/PrintLnArray.java index 78f1221829a..5ef908a23e8 100644 --- a/java/ql/src/Violations of Best Practice/Undesirable Calls/PrintLnArray.java +++ b/java/ql/src/Violations of Best Practice/Undesirable Calls/PrintLnArray.java @@ -1,24 +1,24 @@ -public static void main(String args[]) { - String[] words = {"Who", "is", "John", "Galt"}; - String[][] wordMatrix = {{"There", "is"}, {"no", "spoon"}}; - - // BAD: This implicitly uses 'Object.toString' to convert the contents - // of 'words[]', and prints out something similar to: - // [Ljava.lang.String;@459189e1 - System.out.println(words); - - // GOOD: 'Arrays.toString' calls 'toString' on - // each of the array's elements. The statement prints out: - // [Who, is, John, Galt] - System.out.println(Arrays.toString(words)); - - // ALMOST RIGHT: This calls 'toString' on each of the multi-dimensional - // array's elements. However, because the elements are arrays, the statement - // prints out something similar to: - // [[Ljava.lang.String;@55f33675, [Ljava.lang.String;@527c6768]] - System.out.println(Arrays.toString(wordMatrix)); - - // GOOD: This properly prints out the contents of the multi-dimensional array: - // [[There, is], [no, spoon]] - System.out.println(Arrays.deepToString(wordMatrix)); +public static void main(String args[]) { + String[] words = {"Who", "is", "John", "Galt"}; + String[][] wordMatrix = {{"There", "is"}, {"no", "spoon"}}; + + // BAD: This implicitly uses 'Object.toString' to convert the contents + // of 'words[]', and prints out something similar to: + // [Ljava.lang.String;@459189e1 + System.out.println(words); + + // GOOD: 'Arrays.toString' calls 'toString' on + // each of the array's elements. The statement prints out: + // [Who, is, John, Galt] + System.out.println(Arrays.toString(words)); + + // ALMOST RIGHT: This calls 'toString' on each of the multi-dimensional + // array's elements. However, because the elements are arrays, the statement + // prints out something similar to: + // [[Ljava.lang.String;@55f33675, [Ljava.lang.String;@527c6768]] + System.out.println(Arrays.toString(wordMatrix)); + + // GOOD: This properly prints out the contents of the multi-dimensional array: + // [[There, is], [no, spoon]] + System.out.println(Arrays.deepToString(wordMatrix)); } \ No newline at end of file diff --git a/java/ql/src/plugin.xml b/java/ql/src/plugin.xml index ff451072779..e129a194b56 100644 --- a/java/ql/src/plugin.xml +++ b/java/ql/src/plugin.xml @@ -1,16 +1,16 @@ - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + diff --git a/java/ql/test/library-tests/comments/.gitattributes b/java/ql/test/library-tests/comments/.gitattributes new file mode 100644 index 00000000000..1dd1b58fd4a --- /dev/null +++ b/java/ql/test/library-tests/comments/.gitattributes @@ -0,0 +1 @@ +TestWindows.java eol=crlf diff --git a/java/ql/test/library-tests/comments/TestWindows.java b/java/ql/test/library-tests/comments/TestWindows.java index cdefc327c5b..2409adeed8f 100644 --- a/java/ql/test/library-tests/comments/TestWindows.java +++ b/java/ql/test/library-tests/comments/TestWindows.java @@ -1,22 +1,22 @@ -/** - * A JavaDoc comment - * with multiple lines. - */ -class TestWindows { - /** A JavaDoc comment with a single line. */ - void m() { - // a single-line comment - // another single-line comment - } - - /* A block comment - * with multiple lines. - */ - - /* A block comment with a single line. */ - - // an end-of-line comment with a spurious trailing comment marker */ - // an end-of-line comment with trailing whitespace - //an end-of-line comment without a leading space - void test() {} // an end-of-line comment with preceding code -} +/** + * A JavaDoc comment + * with multiple lines. + */ +class TestWindows { + /** A JavaDoc comment with a single line. */ + void m() { + // a single-line comment + // another single-line comment + } + + /* A block comment + * with multiple lines. + */ + + /* A block comment with a single line. */ + + // an end-of-line comment with a spurious trailing comment marker */ + // an end-of-line comment with trailing whitespace + //an end-of-line comment without a leading space + void test() {} // an end-of-line comment with preceding code +} diff --git a/java/ql/test/library-tests/successors/CloseReaderTest/CloseReaderTest.java b/java/ql/test/library-tests/successors/CloseReaderTest/CloseReaderTest.java index f466bb26518..ab52205adf2 100644 --- a/java/ql/test/library-tests/successors/CloseReaderTest/CloseReaderTest.java +++ b/java/ql/test/library-tests/successors/CloseReaderTest/CloseReaderTest.java @@ -1,25 +1,25 @@ -package successors; - -import java.io.BufferedReader; -import java.io.File; -import java.io.IOException; -import java.io.InputStreamReader; - -public class CloseReaderTest { - public static String readPassword(File keyFile) - { - // TODO: use Console.readPassword() when it's available. - System.out.print("Enter password for " + keyFile - + " (password will not be hidden): "); - System.out.flush(); - BufferedReader stdin = new BufferedReader(new InputStreamReader( - System.in)); - try - { - return stdin.readLine(); - } catch (IOException ex) - { - return null; - } - } -} +package successors; + +import java.io.BufferedReader; +import java.io.File; +import java.io.IOException; +import java.io.InputStreamReader; + +public class CloseReaderTest { + public static String readPassword(File keyFile) + { + // TODO: use Console.readPassword() when it's available. + System.out.print("Enter password for " + keyFile + + " (password will not be hidden): "); + System.out.flush(); + BufferedReader stdin = new BufferedReader(new InputStreamReader( + System.in)); + try + { + return stdin.readLine(); + } catch (IOException ex) + { + return null; + } + } +} diff --git a/java/ql/test/library-tests/successors/LoopVarReadTest/LoopVarReadTest.java b/java/ql/test/library-tests/successors/LoopVarReadTest/LoopVarReadTest.java index 6f8d45e8438..c0a6d9bba4c 100644 --- a/java/ql/test/library-tests/successors/LoopVarReadTest/LoopVarReadTest.java +++ b/java/ql/test/library-tests/successors/LoopVarReadTest/LoopVarReadTest.java @@ -1,16 +1,16 @@ -package successors; - -public class LoopVarReadTest { - public static void testLoop() - { - int x = 2; - for (int y = 0; y < 10; y += x) - { - System.out.println("Foo"); - } - - int q = 10; - - System.out.println("foo"); - } -} +package successors; + +public class LoopVarReadTest { + public static void testLoop() + { + int x = 2; + for (int y = 0; y < 10; y += x) + { + System.out.println("Foo"); + } + + int q = 10; + + System.out.println("foo"); + } +} diff --git a/java/ql/test/library-tests/successors/SaveFileTest/SaveFileTest.java b/java/ql/test/library-tests/successors/SaveFileTest/SaveFileTest.java index 02b4b46cdb2..f07bf930f41 100644 --- a/java/ql/test/library-tests/successors/SaveFileTest/SaveFileTest.java +++ b/java/ql/test/library-tests/successors/SaveFileTest/SaveFileTest.java @@ -1,56 +1,56 @@ -package successors; - -import java.io.File; -import java.io.FileNotFoundException; -import java.io.FileOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; - - -public class SaveFileTest { - public void saveFile(String path, String contentType, - long size, InputStream is) throws FileNotFoundException, - IOException - { - - String savePath = path; - if (path.startsWith("/")) - { - savePath = path.substring(1); - } - - // make sure uploads area exists for this weblog - File dirPath = new File("foo"); - File saveFile = new File(dirPath.getAbsolutePath() + File.separator - + savePath); - - byte[] buffer = new byte[8192]; - int bytesRead = 0; - OutputStream bos = null; - try - { - bos = new FileOutputStream(saveFile); - while ((bytesRead = is.read(buffer, 0, 8192)) != -1) - { - bos.write(buffer, 0, bytesRead); - } - - System.out.println("The file has been written to [" - + saveFile.getAbsolutePath() + "]"); - } catch (Exception e) - { - throw new IOException("ERROR uploading file", e); - } finally - { - try - { - bos.flush(); - bos.close(); - } catch (Exception ignored) - { - } - } - - } -} +package successors; + +import java.io.File; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; + + +public class SaveFileTest { + public void saveFile(String path, String contentType, + long size, InputStream is) throws FileNotFoundException, + IOException + { + + String savePath = path; + if (path.startsWith("/")) + { + savePath = path.substring(1); + } + + // make sure uploads area exists for this weblog + File dirPath = new File("foo"); + File saveFile = new File(dirPath.getAbsolutePath() + File.separator + + savePath); + + byte[] buffer = new byte[8192]; + int bytesRead = 0; + OutputStream bos = null; + try + { + bos = new FileOutputStream(saveFile); + while ((bytesRead = is.read(buffer, 0, 8192)) != -1) + { + bos.write(buffer, 0, bytesRead); + } + + System.out.println("The file has been written to [" + + saveFile.getAbsolutePath() + "]"); + } catch (Exception e) + { + throw new IOException("ERROR uploading file", e); + } finally + { + try + { + bos.flush(); + bos.close(); + } catch (Exception ignored) + { + } + } + + } +} diff --git a/java/ql/test/library-tests/successors/TestBreak/TestBreak.java b/java/ql/test/library-tests/successors/TestBreak/TestBreak.java index e9bfc8aecf7..c4663bb21f3 100644 --- a/java/ql/test/library-tests/successors/TestBreak/TestBreak.java +++ b/java/ql/test/library-tests/successors/TestBreak/TestBreak.java @@ -1,86 +1,86 @@ -package successors; - -public class TestBreak { - public void f() - { - //loop breaks - a: - for (;;) - { - int x = 1; - x = x + 1; - if (x == 1) - { - break; - } else - { - for (int q : new int[20]) - { - if (q == 1) - { - break; - } else - { - break a; - } - } - } - } - int y = 12; - while (true) - { - if (y == 1) - { - break; - } else - { - do - { - if (y == 2) - { - break; - } - y = y + 2; - } while (y == 1); - y = 12; - } - } - y = 13; - - //switch breaks - int x =12; - switch (x) - { - case 1: - x = x + 1; - y = y + 1; - case 2: - x = x + 2; - y = y + 2; - break; - case 3: - case 4: - x = x + 3; - y = y + 4; - break; - case 5: - case 6: - x = x + 5; - y = y + 6; - default: - x = y; - y = x; - } - - //no default - switch(x) - { - case 1: - x = 1; - break; - case 2: - x = 2; - break; - } - } -} +package successors; + +public class TestBreak { + public void f() + { + //loop breaks + a: + for (;;) + { + int x = 1; + x = x + 1; + if (x == 1) + { + break; + } else + { + for (int q : new int[20]) + { + if (q == 1) + { + break; + } else + { + break a; + } + } + } + } + int y = 12; + while (true) + { + if (y == 1) + { + break; + } else + { + do + { + if (y == 2) + { + break; + } + y = y + 2; + } while (y == 1); + y = 12; + } + } + y = 13; + + //switch breaks + int x =12; + switch (x) + { + case 1: + x = x + 1; + y = y + 1; + case 2: + x = x + 2; + y = y + 2; + break; + case 3: + case 4: + x = x + 3; + y = y + 4; + break; + case 5: + case 6: + x = x + 5; + y = y + 6; + default: + x = y; + y = x; + } + + //no default + switch(x) + { + case 1: + x = 1; + break; + case 2: + x = 2; + break; + } + } +} diff --git a/java/ql/test/library-tests/successors/TestContinue/TestContinue.java b/java/ql/test/library-tests/successors/TestContinue/TestContinue.java index 5a0042b3540..77e94e5c7ec 100644 --- a/java/ql/test/library-tests/successors/TestContinue/TestContinue.java +++ b/java/ql/test/library-tests/successors/TestContinue/TestContinue.java @@ -1,59 +1,59 @@ -package successors; - -public class TestContinue { - public void f() - { - //loop breaks - a: - for (int p = 0; p < 10;) - { - int x = 1; - x = x + 1; - if (x == 1) - { - continue; - } else - { - for (int q : new int[20]) - { - if (q == 1) - { - continue; - } else if (q == 2) - { - continue a; - } - q = 12; - } - } - } - int y = 12; - while (y != 13) - { - if (y == 1) - { - continue; - } else - { - do - { - if (y == 2) - { - continue; - } - y = y + 2; - } while (y == 1); - y = 12; - } - y = 15; - } - y = 13; - while (y != 12) - { - if (y != 6) - continue; - else - break; - } - } -} +package successors; + +public class TestContinue { + public void f() + { + //loop breaks + a: + for (int p = 0; p < 10;) + { + int x = 1; + x = x + 1; + if (x == 1) + { + continue; + } else + { + for (int q : new int[20]) + { + if (q == 1) + { + continue; + } else if (q == 2) + { + continue a; + } + q = 12; + } + } + } + int y = 12; + while (y != 13) + { + if (y == 1) + { + continue; + } else + { + do + { + if (y == 2) + { + continue; + } + y = y + 2; + } while (y == 1); + y = 12; + } + y = 15; + } + y = 13; + while (y != 12) + { + if (y != 6) + continue; + else + break; + } + } +} diff --git a/java/ql/test/library-tests/successors/TestFinally/TestFinally.java b/java/ql/test/library-tests/successors/TestFinally/TestFinally.java index 19f67f64a9a..51398bc7b8a 100644 --- a/java/ql/test/library-tests/successors/TestFinally/TestFinally.java +++ b/java/ql/test/library-tests/successors/TestFinally/TestFinally.java @@ -1,150 +1,150 @@ -package successors; - -public class TestFinally { - public void f() - { - int z = 12; - try - { - try - { - System.out.println("Try1"); - if (z == 1) - { - return; - } - try - { - System.out.println("Try1"); - if (z == 1) - { - return; - } - System.out.println("Try2"); - } catch (Exception ex) - { - System.out.println("Exception"); - if (z == 1) - { - return; - } - } finally - { - System.out.println("Finally"); - if (z == 1) - { - return; - } - System.out.println("Finally2"); - } - System.out.println("Try2"); - } catch (Exception ex) - { - System.out.println("Exception"); - try - { - System.out.println("Try1"); - if (z == 1) - { - return; - } - System.out.println("Try2"); - } catch (Exception ex2) - { - System.out.println("Exception"); - if (z == 1) - { - return; - } - } finally - { - System.out.println("Finally"); - if (z == 1) - { - return; - } - System.out.println("Finally2"); - } - if (z == 1) - { - return; - } - } finally - { - System.out.println("Finally"); - if (z == 1) - { - return; - } - System.out.println("Finally2"); - } - System.out.println("Foo"); - int y = 12 + 3; - System.out.println("Bar"); - y = y + 1; - return; - } catch (Exception e) - { - try - { - System.out.println("Try1"); - if (z == 1) - { - return; - } - System.out.println("Try2"); - } catch (Exception ex) - { - System.out.println("Exception"); - if (z == 1) - { - return; - } - } finally - { - System.out.println("Finally"); - if (z == 1) - { - return; - } - System.out.println("Finally2"); - } - int x = 1; - System.out.println("Error: " + e); - x = x + 1; - } finally - { - int y = 12; - System.out.println("Finally"); - y = y + 1; - } - z = z + 1; - - try - { - System.out.println("Try1"); - if (z == 1) - { - return; - } - System.out.println("Try2"); - } catch (Exception ex) - { - System.out.println("Exception"); - if (z == 1) - { - return; - } - } finally - { - System.out.println("Finally"); - if (z == 1) - { - return; - } - System.out.println("Finally2"); - } - - z = z + 2; - } -} +package successors; + +public class TestFinally { + public void f() + { + int z = 12; + try + { + try + { + System.out.println("Try1"); + if (z == 1) + { + return; + } + try + { + System.out.println("Try1"); + if (z == 1) + { + return; + } + System.out.println("Try2"); + } catch (Exception ex) + { + System.out.println("Exception"); + if (z == 1) + { + return; + } + } finally + { + System.out.println("Finally"); + if (z == 1) + { + return; + } + System.out.println("Finally2"); + } + System.out.println("Try2"); + } catch (Exception ex) + { + System.out.println("Exception"); + try + { + System.out.println("Try1"); + if (z == 1) + { + return; + } + System.out.println("Try2"); + } catch (Exception ex2) + { + System.out.println("Exception"); + if (z == 1) + { + return; + } + } finally + { + System.out.println("Finally"); + if (z == 1) + { + return; + } + System.out.println("Finally2"); + } + if (z == 1) + { + return; + } + } finally + { + System.out.println("Finally"); + if (z == 1) + { + return; + } + System.out.println("Finally2"); + } + System.out.println("Foo"); + int y = 12 + 3; + System.out.println("Bar"); + y = y + 1; + return; + } catch (Exception e) + { + try + { + System.out.println("Try1"); + if (z == 1) + { + return; + } + System.out.println("Try2"); + } catch (Exception ex) + { + System.out.println("Exception"); + if (z == 1) + { + return; + } + } finally + { + System.out.println("Finally"); + if (z == 1) + { + return; + } + System.out.println("Finally2"); + } + int x = 1; + System.out.println("Error: " + e); + x = x + 1; + } finally + { + int y = 12; + System.out.println("Finally"); + y = y + 1; + } + z = z + 1; + + try + { + System.out.println("Try1"); + if (z == 1) + { + return; + } + System.out.println("Try2"); + } catch (Exception ex) + { + System.out.println("Exception"); + if (z == 1) + { + return; + } + } finally + { + System.out.println("Finally"); + if (z == 1) + { + return; + } + System.out.println("Finally2"); + } + + z = z + 2; + } +} diff --git a/java/ql/test/library-tests/successors/TestFinallyBreakContinue/TestFinallyBreakContinue.java b/java/ql/test/library-tests/successors/TestFinallyBreakContinue/TestFinallyBreakContinue.java index 480713f60d0..04b232a4ba3 100644 --- a/java/ql/test/library-tests/successors/TestFinallyBreakContinue/TestFinallyBreakContinue.java +++ b/java/ql/test/library-tests/successors/TestFinallyBreakContinue/TestFinallyBreakContinue.java @@ -1,108 +1,108 @@ -package successors; - -public class TestFinallyBreakContinue { - public void f() - { - int x = 1; - a: - for (;;) - { - try - { - if (x == 1) - { - break; - } else - { - continue; - } - } catch (Exception e) - { - if (x == 1) - { - break; - } else - { - continue; - } - } finally - { - System.out.println("finally"); - } - } - - while (true) - { - try - { - try - { - if (x == 1) - { - break; - } else - { - continue; - } - } catch (Exception e) - { - if (x == 1) - { - break; - } else - { - continue; - } - } finally - { - System.out.println("finally"); - } - } catch (Exception e) - { - System.out.println("Exception"); - } finally - { - System.out.println("finally"); - } - } - - b: - do - { - try - { - for (int i : new int[20]) - { - try - { - if (x == 1) - { - break; - } else - { - continue; - } - } catch (Exception e) - { - if (x == 1) - { - break b; - } else - { - continue b; - } - } finally - { - System.out.println("finally"); - } - } - } catch (Exception e) - { - System.out.println("Exception"); - } finally - { - System.out.println("finally"); - } - } while (true); - } -} +package successors; + +public class TestFinallyBreakContinue { + public void f() + { + int x = 1; + a: + for (;;) + { + try + { + if (x == 1) + { + break; + } else + { + continue; + } + } catch (Exception e) + { + if (x == 1) + { + break; + } else + { + continue; + } + } finally + { + System.out.println("finally"); + } + } + + while (true) + { + try + { + try + { + if (x == 1) + { + break; + } else + { + continue; + } + } catch (Exception e) + { + if (x == 1) + { + break; + } else + { + continue; + } + } finally + { + System.out.println("finally"); + } + } catch (Exception e) + { + System.out.println("Exception"); + } finally + { + System.out.println("finally"); + } + } + + b: + do + { + try + { + for (int i : new int[20]) + { + try + { + if (x == 1) + { + break; + } else + { + continue; + } + } catch (Exception e) + { + if (x == 1) + { + break b; + } else + { + continue b; + } + } finally + { + System.out.println("finally"); + } + } + } catch (Exception e) + { + System.out.println("Exception"); + } finally + { + System.out.println("finally"); + } + } while (true); + } +} diff --git a/java/ql/test/library-tests/successors/TestLoopBranch/TestLoopBranch.java b/java/ql/test/library-tests/successors/TestLoopBranch/TestLoopBranch.java index 60906547952..058a9d40275 100644 --- a/java/ql/test/library-tests/successors/TestLoopBranch/TestLoopBranch.java +++ b/java/ql/test/library-tests/successors/TestLoopBranch/TestLoopBranch.java @@ -1,120 +1,120 @@ -package successors; - -public class TestLoopBranch { - int xx = 12; - int yy = 13; - - public void f() - { - int x = 1; - int y = 2; - System.out.println("foo"); - - do - { - System.out.println("bar"); - System.out.println("foobar"); - } while (x == 2); - - { - System.out.println("shazam"); - System.out.println("boogie"); - } - - while (x == 1) - { - System.out.println("wonderland"); - System.out.println("shodan"); - x = x + 1; - } - - for (int i = 0; i < 10; i++) - { - System.out.println("rapture"); - y = x - 2; - } - - ; - ; - - for (int j : new int[20]) - { - System.out.println("Zero : " + j); - j = j + x; - } - - if (y == -1) - { - System.out.println("i squared"); - } - - if (x == 42) - { - System.out.println("rat"); - x = 6 * 9; - } else - { - System.out.println("arr"); - x = y * x; - return; - } - - switch (x) - { - case 1: - x = x + 1; - y = y + 1; - case 2: - x = x + 2; - y = y + 2; - break; - case 3: - case 4: - x = x + 3; - y = y + 4; - break; - case 5: - case 6: - x = x + 5; - y = y + 6; - default: - x = y; - y = x; - } - - //no default - switch(x) - { - case 1: - x = 1; - break; - case 2: - x = 2; - break; - } - - Comparable b = new Comparable() { - @Override - public int compareTo(String o) - { - return 0; - } - }; - b.compareTo("Foo"); - - x = x + y; - return; - } - - public TestLoopBranch() - { - xx = 33; - yy = 44; - } - - public TestLoopBranch(int i) - { - xx = i; - yy = i; - } +package successors; + +public class TestLoopBranch { + int xx = 12; + int yy = 13; + + public void f() + { + int x = 1; + int y = 2; + System.out.println("foo"); + + do + { + System.out.println("bar"); + System.out.println("foobar"); + } while (x == 2); + + { + System.out.println("shazam"); + System.out.println("boogie"); + } + + while (x == 1) + { + System.out.println("wonderland"); + System.out.println("shodan"); + x = x + 1; + } + + for (int i = 0; i < 10; i++) + { + System.out.println("rapture"); + y = x - 2; + } + + ; + ; + + for (int j : new int[20]) + { + System.out.println("Zero : " + j); + j = j + x; + } + + if (y == -1) + { + System.out.println("i squared"); + } + + if (x == 42) + { + System.out.println("rat"); + x = 6 * 9; + } else + { + System.out.println("arr"); + x = y * x; + return; + } + + switch (x) + { + case 1: + x = x + 1; + y = y + 1; + case 2: + x = x + 2; + y = y + 2; + break; + case 3: + case 4: + x = x + 3; + y = y + 4; + break; + case 5: + case 6: + x = x + 5; + y = y + 6; + default: + x = y; + y = x; + } + + //no default + switch(x) + { + case 1: + x = 1; + break; + case 2: + x = 2; + break; + } + + Comparable b = new Comparable() { + @Override + public int compareTo(String o) + { + return 0; + } + }; + b.compareTo("Foo"); + + x = x + y; + return; + } + + public TestLoopBranch() + { + xx = 33; + yy = 44; + } + + public TestLoopBranch(int i) + { + xx = i; + yy = i; + } } \ No newline at end of file diff --git a/java/ql/test/library-tests/successors/TestThrow/TestThrow.java b/java/ql/test/library-tests/successors/TestThrow/TestThrow.java index 78df2a3595f..58098fc6505 100644 --- a/java/ql/test/library-tests/successors/TestThrow/TestThrow.java +++ b/java/ql/test/library-tests/successors/TestThrow/TestThrow.java @@ -1,135 +1,135 @@ -package successors; - -import java.io.IOException; -import java.security.InvalidParameterException; - -public class TestThrow { - private TestThrow() throws IOException - { - } - - private void thrower() throws InvalidParameterException - { - } - - public void f() throws Exception - { - int z = 0; - try - { - throw new RuntimeException(); - } catch (RuntimeException e) - { - z = 1; - } catch (Exception e) - { - z = 2; - } - - z = -1; - - try - { - if (z == 1) - { - throw new RuntimeException(); - } else if (z == 2) - { - throw new Exception(); - } else if (z == 3) - { - new TestThrow(); - } else - { - thrower(); - } - } catch (RuntimeException e) - { - z = 1; - } finally - { - z = 2; - } - - z = -1; - - try - { - if (z == 1) - { - throw new Exception(); - } - else if (z == 2) - { - new TestThrow(); - } else - { - thrower(); - } - } catch (RuntimeException e) - { - z = 1; - } - - z = -1; - - try - { - if (z == 1) - throw new Exception(); - } finally - { - z = 1; - } - - try - { - try - { - if (z == 1) - { - throw new Exception(); - } else if (z == 2) - { - throw new RuntimeException(); - } else - { - throw new IOException("Foo bar", null); - } - } catch (RuntimeException e) - { - z = 1; - } - try - { - z = -2; - } finally - { - if (z == 1) - { - throw new Exception(); - } else if (z == 2) - { - throw new RuntimeException(); - } else if (z == 3) - { - throw new IOException("Foo bar", null); - } - } - } catch (IOException e) - { - z = 2; - } - finally - { - z = 3; - } - - if (z == 1) - { - throw new Exception(); - } - - z = -1; - } -} +package successors; + +import java.io.IOException; +import java.security.InvalidParameterException; + +public class TestThrow { + private TestThrow() throws IOException + { + } + + private void thrower() throws InvalidParameterException + { + } + + public void f() throws Exception + { + int z = 0; + try + { + throw new RuntimeException(); + } catch (RuntimeException e) + { + z = 1; + } catch (Exception e) + { + z = 2; + } + + z = -1; + + try + { + if (z == 1) + { + throw new RuntimeException(); + } else if (z == 2) + { + throw new Exception(); + } else if (z == 3) + { + new TestThrow(); + } else + { + thrower(); + } + } catch (RuntimeException e) + { + z = 1; + } finally + { + z = 2; + } + + z = -1; + + try + { + if (z == 1) + { + throw new Exception(); + } + else if (z == 2) + { + new TestThrow(); + } else + { + thrower(); + } + } catch (RuntimeException e) + { + z = 1; + } + + z = -1; + + try + { + if (z == 1) + throw new Exception(); + } finally + { + z = 1; + } + + try + { + try + { + if (z == 1) + { + throw new Exception(); + } else if (z == 2) + { + throw new RuntimeException(); + } else + { + throw new IOException("Foo bar", null); + } + } catch (RuntimeException e) + { + z = 1; + } + try + { + z = -2; + } finally + { + if (z == 1) + { + throw new Exception(); + } else if (z == 2) + { + throw new RuntimeException(); + } else if (z == 3) + { + throw new IOException("Foo bar", null); + } + } + } catch (IOException e) + { + z = 2; + } + finally + { + z = 3; + } + + if (z == 1) + { + throw new Exception(); + } + + z = -1; + } +} diff --git a/java/ql/test/library-tests/successors/TestTryCatch/TestTryCatch.java b/java/ql/test/library-tests/successors/TestTryCatch/TestTryCatch.java index 38b320ae8e1..09466051023 100644 --- a/java/ql/test/library-tests/successors/TestTryCatch/TestTryCatch.java +++ b/java/ql/test/library-tests/successors/TestTryCatch/TestTryCatch.java @@ -1,44 +1,44 @@ -package successors; - -public class TestTryCatch { - public void f() - { - try - { - System.out.println("Foo"); - int y = 12 + 3; - System.out.println("Bar"); - y = y + 1; - } catch (Exception e) - { - int x = 1; - System.out.println("Error: " + e); - x = x + 1; - return; - } finally - { - int y = 12; - System.out.println("Finally"); - y = y + 1; - } - int z = 12; - z = z + 1; - - for (int q = 0; q < 10; q++) - { - try - { - System.out.println("Foo"); - int y = 12 + 3; - System.out.println("Bar"); - y = y + 1; - } catch (RuntimeException e) - { - int x = 1; - System.out.println("Error: " + e); - x = x + 1; - } - } - z = z + 2; - } -} +package successors; + +public class TestTryCatch { + public void f() + { + try + { + System.out.println("Foo"); + int y = 12 + 3; + System.out.println("Bar"); + y = y + 1; + } catch (Exception e) + { + int x = 1; + System.out.println("Error: " + e); + x = x + 1; + return; + } finally + { + int y = 12; + System.out.println("Finally"); + y = y + 1; + } + int z = 12; + z = z + 1; + + for (int q = 0; q < 10; q++) + { + try + { + System.out.println("Foo"); + int y = 12 + 3; + System.out.println("Bar"); + y = y + 1; + } catch (RuntimeException e) + { + int x = 1; + System.out.println("Error: " + e); + x = x + 1; + } + } + z = z + 2; + } +} diff --git a/java/ql/test/query-tests/AlertSuppression/.gitattributes b/java/ql/test/query-tests/AlertSuppression/.gitattributes new file mode 100644 index 00000000000..1dd1b58fd4a --- /dev/null +++ b/java/ql/test/query-tests/AlertSuppression/.gitattributes @@ -0,0 +1 @@ +TestWindows.java eol=crlf diff --git a/java/ql/test/query-tests/AlertSuppression/TestWindows.java b/java/ql/test/query-tests/AlertSuppression/TestWindows.java index da2539a3083..07845e3c0a5 100644 --- a/java/ql/test/query-tests/AlertSuppression/TestWindows.java +++ b/java/ql/test/query-tests/AlertSuppression/TestWindows.java @@ -1,28 +1,28 @@ -class TestWindows {} // lgtm -// lgtm[java/confusing-method-name] -// lgtm[java/confusing-method-name, java/non-short-circuit-evaluation] -// lgtm[@tag:exceptions] -// lgtm[@tag:exceptions,java/confusing-method-name] -// lgtm[@expires:2017-06-11] -// lgtm[java/confusing-method-name] does not seem confusing despite alert by lgtm -// lgtm: blah blah -// lgtm blah blah #falsepositive -//lgtm [java/confusing-method-name] -/* lgtm */ -// lgtm[] -// lgtmfoo -//lgtm -// lgtm -// lgtm [java/confusing-method-name] -// foolgtm[java/confusing-method-name] -// foolgtm -// foo; lgtm -// foo; lgtm[java/confusing-method-name] -// foo lgtm -// foo lgtm[java/confusing-method-name] -// foo lgtm bar -// foo lgtm[java/confusing-method-name] bar -// LGTM! -// LGTM[java/confusing-method-name] -//lgtm[java/confusing-method-name] and lgtm[java/non-short-circuit-evaluation] -//lgtm[java/confusing-method-name]; lgtm +class TestWindows {} // lgtm +// lgtm[java/confusing-method-name] +// lgtm[java/confusing-method-name, java/non-short-circuit-evaluation] +// lgtm[@tag:exceptions] +// lgtm[@tag:exceptions,java/confusing-method-name] +// lgtm[@expires:2017-06-11] +// lgtm[java/confusing-method-name] does not seem confusing despite alert by lgtm +// lgtm: blah blah +// lgtm blah blah #falsepositive +//lgtm [java/confusing-method-name] +/* lgtm */ +// lgtm[] +// lgtmfoo +//lgtm +// lgtm +// lgtm [java/confusing-method-name] +// foolgtm[java/confusing-method-name] +// foolgtm +// foo; lgtm +// foo; lgtm[java/confusing-method-name] +// foo lgtm +// foo lgtm[java/confusing-method-name] +// foo lgtm bar +// foo lgtm[java/confusing-method-name] bar +// LGTM! +// LGTM[java/confusing-method-name] +//lgtm[java/confusing-method-name] and lgtm[java/non-short-circuit-evaluation] +//lgtm[java/confusing-method-name]; lgtm diff --git a/javascript/ql/src/plugin.xml b/javascript/ql/src/plugin.xml index 206533511dc..19bcfc22249 100644 --- a/javascript/ql/src/plugin.xml +++ b/javascript/ql/src/plugin.xml @@ -1,16 +1,16 @@ - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + +