Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Annotating classes of util package #1

Open
wants to merge 41 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
adde8f7
annotating
Kakarot-SSJ4 Jul 12, 2019
a1e270d
MultidimesnionalCounter
Kakarot-SSJ4 Jul 12, 2019
f0c7f44
pom.xml
Kakarot-SSJ4 Jul 12, 2019
91e7996
unnecessary changes
Kakarot-SSJ4 Jul 12, 2019
5187fb2
pom
Kakarot-SSJ4 Jul 12, 2019
9170b66
recitfying
Kakarot-SSJ4 Jul 12, 2019
0129347
removed unnecessary changes
Kakarot-SSJ4 Jul 12, 2019
2940ec6
annotating
Kakarot-SSJ4 Jul 15, 2019
a87a7fe
made suggested changes
Kakarot-SSJ4 Jul 16, 2019
428f5fd
annotating FastMath.java
Kakarot-SSJ4 Jul 19, 2019
9ec2c44
OpenIntToDoubleHashMap.java
Kakarot-SSJ4 Jul 21, 2019
d9f54c8
OpenIntToFieldHashMap.java
Kakarot-SSJ4 Jul 22, 2019
1bf39c6
annotating
Kakarot-SSJ4 Jul 22, 2019
a57e7d0
fixing
Kakarot-SSJ4 Jul 22, 2019
6cee18b
fixing
Kakarot-SSJ4 Jul 22, 2019
dfe72f2
made suggested changes
Kakarot-SSJ4 Jul 23, 2019
a74ac32
made suggested changes
Kakarot-SSJ4 Jul 23, 2019
c608b71
made suggested changes
Kakarot-SSJ4 Jul 23, 2019
00e9867
made suggested changes
Kakarot-SSJ4 Jul 23, 2019
112248e
annotated FastMathCalc.java
Kakarot-SSJ4 Jul 23, 2019
f3cfb7e
checker version 2.9.0
Kakarot-SSJ4 Jul 24, 2019
855cf84
made suggested changes
Kakarot-SSJ4 Jul 25, 2019
e96b1a7
made suggested changes
Kakarot-SSJ4 Jul 25, 2019
ca5ae72
made suggested changes
Kakarot-SSJ4 Jul 25, 2019
abee324
Merge branch 'annotated-2' into annotated-3
Kakarot-SSJ4 Jul 25, 2019
5bbf6e7
made suggested changes
Kakarot-SSJ4 Jul 25, 2019
915e208
Merge branch 'annotated' into annotated-2
Kakarot-SSJ4 Jul 25, 2019
39f4314
Merge branch 'annotated-2' into annotated-3
Kakarot-SSJ4 Jul 25, 2019
dbb06b9
message
Kakarot-SSJ4 Jul 25, 2019
4e439f5
openjdk
Kakarot-SSJ4 Jul 31, 2019
53d47ef
IntRange
Kakarot-SSJ4 Aug 16, 2019
473c6de
@NonNegative
Kakarot-SSJ4 Aug 16, 2019
a59a8c5
error handled
Kakarot-SSJ4 Aug 16, 2019
07fe742
made suggested changes
Kakarot-SSJ4 Aug 16, 2019
566f7af
Merge pull request #3 from Kakarot-SSJ4/annotated-3
Kakarot-SSJ4 Aug 21, 2019
1102c66
Merge branch 'annotated' into annotated-2
Kakarot-SSJ4 Aug 21, 2019
a91c465
Merge pull request #1 from Kakarot-SSJ4/annotated-2
Kakarot-SSJ4 Aug 21, 2019
aed29ed
recctifying errors
Kakarot-SSJ4 Aug 21, 2019
dd1859e
resolved all the errors
Kakarot-SSJ4 Aug 21, 2019
623e052
skip test files
Kakarot-SSJ4 Aug 21, 2019
2cf074f
made suggested changes
Kakarot-SSJ4 Aug 29, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 90 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,19 @@
</contributors>

<dependencies>

<dependency>
<groupId>org.checkerframework</groupId>
<artifactId>checker-qual</artifactId>
<version>2.8.2</version>
</dependency>

<dependency>
<groupId>org.checkerframework</groupId>
<artifactId>jdk8</artifactId>
<version>2.8.2</version>
</dependency>

<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-statistics-distribution</artifactId>
Expand Down Expand Up @@ -460,6 +473,7 @@
</dependencies>

<properties>
<annotatedJdk>${org.checkerframework:jdk8:jar}</annotatedJdk>
<!-- Do not change: "math" is the name of the component even if the
name of the base package evolves with major release numbers
(see "commons.osgi.symbolicName", below). -->
Expand Down Expand Up @@ -516,6 +530,47 @@

<build>
<plugins>
<plugin>
<!-- This plugin will set properties values using dependency information -->

Choose a reason for hiding this comment

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

Fix indentation in this file.

<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
<executions>
<execution>
<goals>
<goal>properties</goal>
</goals>
</execution>
</executions>
</plugin>
<plugin>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.6.1</version>
<configuration>
<source>1.8</source>
<target>1.8</target>
<compilerArguments>
<Xmaxerrs>10000</Xmaxerrs>
<Xmaxwarns>10000</Xmaxwarns>
</compilerArguments>
<annotationProcessorPaths>
<path>
<groupId>org.checkerframework</groupId>
<artifactId>checker</artifactId>
<version>2.8.2</version>
</path>
</annotationProcessorPaths>
<annotationProcessors>
<!-- Add all the checkers you want to enable here -->
<annotationProcessor>org.checkerframework.checker.index.IndexChecker</annotationProcessor>
</annotationProcessors>
<compilerArgs>
<!-- location of the annotated JDK, which comes from a Maven dependency -->
<arg>-Xbootclasspath/p:${annotatedJdk}</arg>
<arg>-AonlyDefs=^org\.apache\.commons\.math4\.util\.</arg>

Choose a reason for hiding this comment

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

Can you also add -ArequirePrefixInWarningSuppressions and -AwarnUnneededSuppressions as in commons-lang?

</compilerArgs>
</configuration>
</plugin>

<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
Expand Down Expand Up @@ -1048,5 +1103,40 @@
</plugins>
</build>
</profile>
<profile>
<id>mathjax-java8</id>
<activation><jdk>[1.8,)</jdk></activation>
<properties>
<!-- MathJax requires additional Javadoc qualifier for Java8+ -->
<allowscript.javadoc.qualifier>--allow-script-in-comments</allowscript.javadoc.qualifier>
</properties>
</profile>
<profile>
<id>checker</id>
<dependencies>
<dependency>
<groupId>org.checkerframework</groupId>
<artifactId>checker</artifactId>
<version>2.8.2</version>
<scope>provided</scope>
</dependency>
</dependencies>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
<version>2.10</version>
<executions>
<execution>
<goals>
<goal>properties</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
</profiles>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@

import org.apache.commons.math4.exception.MathIllegalArgumentException;

import org.checkerframework.checker.index.qual.IndexFor;
import org.checkerframework.checker.index.qual.LTEqLengthOf;
import org.checkerframework.checker.index.qual.Positive;
import org.checkerframework.checker.index.qual.LessThan;


/**
* A mid point strategy based on the average of begin and end indices.
Expand All @@ -38,10 +43,11 @@ public class CentralPivotingStrategy implements PivotingStrategyInterface, Seria
* @throws MathIllegalArgumentException when indices exceeds range
*/
@Override
public int pivotIndex(final double[] work, final int begin, final int end)
@SuppressWarnings("index:return.type.incompatible") // #1: begin + (end - begin)/2 = (begin + end)/2 where begin and end are @IndexFor("work"), hence, (begin + end)/2 is @IndexFor("work")
public @IndexFor("#1") int pivotIndex(final double[] work, final @LessThan("#3") @IndexFor("#1") int begin, final @Positive @LTEqLengthOf("#1") int end)
throws MathIllegalArgumentException {
MathArrays.verifyValues(work, begin, end-begin);
return begin + (end - begin)/2;
return begin + (end - begin)/2; // #1
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
import org.apache.commons.numbers.combinatorics.Factorial;
import org.apache.commons.numbers.combinatorics.BinomialCoefficient;

import org.checkerframework.checker.index.qual.NonNegative;
import org.checkerframework.checker.index.qual.LessThan;

/**
* Combinatorial utilities.
*
Expand Down Expand Up @@ -57,7 +60,12 @@ private CombinatoricsUtils() {}
* k between 20 and n-2 (S(n,n-1) is handled specifically and does not overflow)
* @since 3.1
*/
public static long stirlingS2(final int n, final int k)
@SuppressWarnings("index:array.access.unsafe.high") /*
#1: i starts from 1, hence stirlingS2 has minimum 2 columns
#2: stirlingS2 has i + 1 columns, and j < i is checked
#3: k <= n as annotated and n < stirlingS2.length as annotated
*/
public static long stirlingS2(final @NonNegative int n, final @NonNegative @LessThan("#1 + 1") int k)
throws NotPositiveException, NumberIsTooLargeException, MathArithmeticException {
if (k < 0) {
throw new NotPositiveException(k);
Expand All @@ -78,12 +86,12 @@ public static long stirlingS2(final int n, final int k)
stirlingS2 = new long[maxIndex][];
stirlingS2[0] = new long[] { 1l };
for (int i = 1; i < stirlingS2.length; ++i) {
stirlingS2[i] = new long[i + 1];
stirlingS2[i][0] = 0;
stirlingS2[i][1] = 1;
stirlingS2[i] = new long[i + 1]; // #0.1
stirlingS2[i][0] = 0; // #1
stirlingS2[i][1] = 1; // #1
stirlingS2[i][i] = 1;
for (int j = 2; j < i; ++j) {
stirlingS2[i][j] = j * stirlingS2[i - 1][j] + stirlingS2[i - 1][j - 1];
stirlingS2[i][j] = j * stirlingS2[i - 1][j] + stirlingS2[i - 1][j - 1]; // #2
}
}

Expand All @@ -94,7 +102,7 @@ public static long stirlingS2(final int n, final int k)

if (n < stirlingS2.length) {
// the number is in the small cache
return stirlingS2[n][k];
return stirlingS2[n][k]; // #3
} else {
// use explicit formula to compute the number without caching it
if (k == 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ public static void parseAndIgnoreWhitespace(final String source,
* @param pos input/output parsing parameter.
* @return the first non-whitespace character.
*/
@SuppressWarnings("index:argument.type.incompatible") // #1: index < n (source.length) as checked by the condition of the loop and for the first time by the if statement
public static char parseNextCharacter(final String source,
final ParsePosition pos) {
int index = pos.getIndex();
Expand All @@ -84,7 +85,7 @@ public static char parseNextCharacter(final String source,
if (index < n) {
char c;
do {
c = source.charAt(index++);
c = source.charAt(index++); // #1
} while (Character.isWhitespace(c) && index < n);
pos.setIndex(index);

Expand All @@ -105,6 +106,7 @@ public static char parseNextCharacter(final String source,
* @param pos input/output parsing parameter.
* @return the special number.
*/
@SuppressWarnings("index:argument.type.incompatible") // #1: startIndex is @NonNegative, and as startIndex + n (@NonNegative) < source.length(), startIndex < source.length
private static Number parseNumber(final String source, final double value,
final ParsePosition pos) {
Number ret = null;
Expand All @@ -118,7 +120,7 @@ private static Number parseNumber(final String source, final double value,
final int startIndex = pos.getIndex();
final int endIndex = startIndex + n;
if (endIndex < source.length() &&
source.substring(startIndex, endIndex).compareTo(sb.toString()) == 0) {
source.substring(startIndex, endIndex).compareTo(sb.toString()) == 0) { // #1
ret = Double.valueOf(value);
pos.setIndex(endIndex);
}
Expand Down Expand Up @@ -166,6 +168,7 @@ public static Number parseNumber(final String source, final NumberFormat format,
* @param pos input/output parsing parameter.
* @return true if the expected string was there
*/
@SuppressWarnings("index:argument.type.incompatible") // The substring function is called only when the previous two condition fail, i.e, only when startIndex < source.length() && endIndex <= source.length()
public static boolean parseFixedstring(final String source,
final String expected,
final ParsePosition pos) {
Expand All @@ -174,7 +177,7 @@ public static boolean parseFixedstring(final String source,
final int endIndex = startIndex + expected.length();
if ((startIndex >= source.length()) ||
(endIndex > source.length()) ||
(source.substring(startIndex, endIndex).compareTo(expected) != 0)) {
(source.substring(startIndex, endIndex).compareTo(expected) != 0)) { // #1
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would expect this call to substring to not issue an error, because the expected type for both arguments is IndexOrHigh - which should both be established by the conditionals, as your suppress warning comment notes. Can you write a test case like this code and submit a bug? Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

The explanation @SuppressWarnings was wrong, it actually shows array.access.unsafe.low error as it cannot recognise pos.getIndex() as @NonNegative.
The PR with ParsePosition annotation is open, so I have provided a link in the comment.

// set index back to start, error index should be the start index
pos.setIndex(startIndex);
pos.setErrorIndex(startIndex);
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/org/apache/commons/math4/util/DoubleArray.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
*/
package org.apache.commons.math4.util;

import org.checkerframework.checker.index.qual.NonNegative;


/**
* Provides a standard interface for double arrays. Allows different
Expand Down Expand Up @@ -57,7 +59,7 @@ public interface DoubleArray {
* @throws ArrayIndexOutOfBoundsException if <code>index</code> is less than
* zero.
*/
void setElement(int index, double value);
void setElement(@NonNegative int index, double value);

/**
* Adds an element to the end of this expandable array
Expand Down
29 changes: 19 additions & 10 deletions src/main/java/org/apache/commons/math4/util/KthSelector.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@

import org.apache.commons.math4.exception.NullArgumentException;

import org.checkerframework.checker.index.qual.IndexFor;
import org.checkerframework.checker.index.qual.NonNegative;
import org.checkerframework.checker.index.qual.IndexOrHigh;


/**
* A Simple K<sup>th</sup> selector implementation to pick up the
Expand Down Expand Up @@ -76,7 +80,12 @@ public PivotingStrategyInterface getPivotingStrategy() {
* @param k the index whose value in the array is of interest
* @return K<sup>th</sup> value
*/
public double select(final double[] work, final int[] pivotsHeap, final int k) {
@SuppressWarnings({"index:array.access.unsafe.low", "index:argument.type.incompatible"}) /*
#1: node is always @NonNegative as it is changed only in #0.1 where it is minimum of 2*node + 1(or 2) and either pivotsHeap.length or end

Choose a reason for hiding this comment

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

Then node should be annotated @NonNegative. Possibly begin and end as well.

pivotsHeap.length is @NonNegative as it is a length, end is also @NonNegative as it is initialized 0 and changed only in #0.2 where end = pivot >= k that is @NonNegative
#3: begin and end are @NonNegative as t is assign
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does "as t is assign" mean?

*/
public double select(final double[] work, final int[] pivotsHeap, final @IndexFor("#1") int k) {
int begin = 0;
int end = work.length;
int node = 0;
Expand All @@ -85,15 +94,15 @@ public double select(final double[] work, final int[] pivotsHeap, final int k) {
final int pivot;

if (usePivotsHeap && node < pivotsHeap.length &&
pivotsHeap[node] >= 0) {
pivotsHeap[node] >= 0) { // #1
// the pivot has already been found in a previous call
// and the array has already been partitioned around it
pivot = pivotsHeap[node];
pivot = pivotsHeap[node]; // #1
} else {
// select a pivot and partition work array around it
pivot = partition(work, begin, end, pivotingStrategy.pivotIndex(work, begin, end));
pivot = partition(work, begin, end, pivotingStrategy.pivotIndex(work, begin, end)); // #2
if (usePivotsHeap && node < pivotsHeap.length) {
pivotsHeap[node] = pivot;
pivotsHeap[node] = pivot; // #1
}
}

Expand All @@ -102,15 +111,15 @@ public double select(final double[] work, final int[] pivotsHeap, final int k) {
return work[k];
} else if (k < pivot) {
// the element is in the left partition
end = pivot;
node = FastMath.min(2 * node + 1, usePivotsHeap ? pivotsHeap.length : end);
end = pivot; // #0.2
node = FastMath.min(2 * node + 1, usePivotsHeap ? pivotsHeap.length : end); // #0.1
} else {
// the element is in the right partition
begin = pivot + 1;
node = FastMath.min(2 * node + 2, usePivotsHeap ? pivotsHeap.length : end);
node = FastMath.min(2 * node + 2, usePivotsHeap ? pivotsHeap.length : end); // #0.1
}
}
Arrays.sort(work, begin, end);
Arrays.sort(work, begin, end); // #3
return work[k];
}

Expand All @@ -125,7 +134,7 @@ public double select(final double[] work, final int[] pivotsHeap, final int k) {
* @param pivot initial index of the pivot
* @return index of the pivot after partition
*/
private int partition(final double[] work, final int begin, final int end, final int pivot) {
private @NonNegative int partition(final double[] work, final @IndexFor("#1") int begin, final @IndexOrHigh("#1") int end, final @IndexFor("#1") int pivot) {

final double value = work[pivot];
work[pivot] = work[begin];
Expand Down
Loading