Add Index Checker annotations #2

kelloggm · 2018-05-01T22:49:02Z

This pull request includes all of the Index Checker annotations for the JFreeChart case study. When compiling (i.e. running mvn compile), the Index Checker is run automatically (using the version of the checker from Maven Central).

I had to revert the old commit that added annotations in so that this can be merged cleanly. I've also removed all traces of the old ant build files.

The annotated version of JFreeChart is version 1.5.0.

…ing remaining errors

…except those caused by missing annotations in javax.swing classes

… and need to be investigated further; everything else has either been removed or suppressed

…ndex

…erer.

panacekcz

I just quickly looked through the diff, looks good to me, just nitpicks: found some annotations still in comments (see review comments), sometimes inconsistent whitespace (I didn't mark that).

panacekcz · 2018-05-02T12:44:50Z

src/main/java/org/jfree/chart/date/SerialDate.java

@@ -423,8 +433,10 @@ public static int stringToMonthCode(String s) {
                }
            }
        }
+        @SuppressWarnings({"index", "value"}) // the loop above only ensures that 1 <= result <= 12 if s is an actual month name
+        @IntRange(from = 1, to =12) int toReturn = result;


Should be @IntRange(from = -1, to = 12)?

No. The if loop above ensures that the value is 1 to 12 if the variable s is a valid month name, which is a precondition of the function.

panacekcz · 2018-05-02T12:46:32Z

src/main/java/org/jfree/chart/date/SpreadsheetDate.java

@@ -76,7 +82,7 @@
     * @param month  the month (in the range 1 to 12).
     * @param year  the year (in the range 1900 to 9999).
     */
-    public SpreadsheetDate(int day, int month, int year) {
+    public SpreadsheetDate(@IntRange(from = 1, to = 31) int day, /*@IntRange(from = 1, to = 12)*/ int month, @IntRange(from = 1, to = 9999) int year) {


Annotation in comment.

panacekcz · 2018-05-02T12:47:42Z

src/main/java/org/jfree/chart/date/SpreadsheetDate.java

@@ -425,16 +445,22 @@ else if (include == SerialDate.INCLUDE_SECOND) {
     *
     * @return the serial number from the day, month and year.
     */
-    private int calcSerial(int d, int m, int y) {
+    private @IntRange(from = 2, to = 2958465) int calcSerial(/*@IntRange(from = 1, to = 31)*/ int d, /*@IntRange(from=1,to=12)*/ int m, @IntRange(from = 1900, to = 9999) int y) {


Annotations in comments.

panacekcz · 2018-05-02T13:09:25Z

src/main/java/org/jfree/data/DataUtils.java

@@ -238,10 +241,11 @@ public static double calculateRowTotal(Values2D data, int row,
     *
     * @return An array of {@code double}.
     */
-    public static Number[][] createNumberArray2D(double[][] data) {
+    @SuppressWarnings("value") // the result array is manifestly the same size as the parameter
+    public static Number @SameLen("#1") /*@PolyValue*/ [][] createNumberArray2D(double @PolyValue [][] data) {


Annotation in comments.

panacekcz · 2018-05-02T13:23:04Z

src/main/java/org/jfree/data/statistics/Regression.java

@@ -63,7 +70,7 @@
     *
     * @return The parameters.
     */
-    public static double[] getOLSRegression(double[][] data) {
+    public static double @ArrayLen(2) [] getOLSRegression(double /*@MinLen(2)*/ [] @MinLen(2) [] data) {


Annotation in comment.

panacekcz · 2018-05-02T13:25:59Z

src/main/java/org/jfree/data/statistics/Regression.java

@@ -150,7 +157,7 @@
     *
     * @return The parameters.
     */
-    public static double[] getPowerRegression(double[][] data) {
+    public static double @ArrayLen(2) [] getPowerRegression(double /*@MinLen(2)*/ [] @MinLen(2) [] data) {


Annotation in comments.

panacekcz · 2018-05-02T13:26:45Z

src/main/java/org/jfree/data/statistics/Statistics.java

@@ -327,7 +332,7 @@ public static double getStdDev(Number[] data) {
     *
     * @return A double array with the intercept in [0] and the slope in [1].
     */
-    public static double[] getLinearFit(Number[] xData, Number[] yData) {
+    public static double @ArrayLen(2) [] getLinearFit(Number /*@SameLen("#2")*/ [] xData, Number @SameLen("#1") [] yData) {


Annotation in comment.

panacekcz · 2018-05-02T13:27:12Z

src/main/java/org/jfree/data/statistics/Statistics.java

@@ -439,8 +444,8 @@ public static double getCorrelation(Number[] data1, Number[] data2) {
     * @return A double[][] the length of the data set in the first dimension,
     *         with two doubles for x and y in the second dimension
     */
-    public static double[][] getMovingAverage(Number[] xData, Number[] yData,
-            int period) {
+    public static double[] @ArrayLen(2) [] getMovingAverage(Number /*@SameLen("#2")*/ [] xData, Number @SameLen("#1") [] yData,


Annotation in comment.

panacekcz · 2018-05-02T17:19:10Z

src/main/java/org/jfree/data/xy/DefaultHighLowDataset.java

-            double[] high, double[] low, double[] open, double[] close,
-            double[] volume) {
+    @SuppressWarnings("samelen") // While initializing object, SameLen invariants between the fields will be broken (because one field must be initialized before the others). The annotations on the parameters guarantee the invariant holds after the constructor finishes executing.
+    public DefaultHighLowDataset(Comparable seriesKey, Date @SameLen({"#2", "#3", "#4", "#4", "#5", "#6", "#7"}) [] date,


Duplicate "#4"

panacekcz · 2018-05-02T17:23:52Z

src/test/java/org/jfree/chart/GanttChartTest.java

@@ -271,7 +271,7 @@ public static IntervalCategoryDataset createDataset() {
     *
     * @return a date.
     */
-    private static Date date(int day, int month, int year) {
+    private static Date date(@IntRange(from = 1, to = 31) int day, /*@IntRange(from = 1, to = 12)*/ int month, @IntRange(from = 1900, to = 9999) int year) {


Annotation in comment.

mernst

The standard Maven target is mvn package.
That works for upstream JFreeChart, but it fails for this pull request.
Please make mvn package work, or document some other command that builds all the artifacts.

I have reviewed about 20% of the changes, but many of my comments apply throughout the codebase. Please make those changes and I will re-review.

mernst · 2018-05-05T12:38:44Z

.gitignore

@@ -2,10 +2,12 @@
 /target/
 /javadoc/

+build/


Why is this change needed?

mernst · 2018-05-05T12:39:20Z

.gitignore

Please don't make unnecessary whitespace changes. It pollutes the diffs.

mernst · 2018-05-05T12:39:41Z

.travis.yml

+sudo: false
+script:
+  - mvn clean
+  - travis_wait mvn compile


Add newline at end of files, when creating a new file.

mernst · 2018-05-05T12:40:10Z

.travis.yml

+language: java
+sudo: false
+script:
+  - mvn clean


Why is this needed? A Travis job always starts out from a clean clone.

mernst · 2018-05-05T12:40:31Z

pom.xml

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


Update to latest version.

mernst · 2018-05-05T13:22:38Z

src/main/java/org/jfree/chart/date/SerialDate.java

@@ -479,9 +490,10 @@ else if ((yyyy % 100) == 0) {
     *
     * @param yyyy  the year (in the range 1900 to 9999).
     *
-     * @return the number of leap years from 1900 to the specified year.
+     * @return the number of leap years from 1900 to the specified year (if year is 9999, 1964 leap years).


Comment is too telegraphic. Add a few words to clarify.

mernst · 2018-05-05T13:22:56Z

src/main/java/org/jfree/chart/date/SerialDate.java

     */
-    public static int leapYearCount(int yyyy) {
+    @SuppressWarnings({"index", "value"}) // imprecision wrt ranges
+    public static @IntRange(from = 0, to = 1964) int leapYearCount(@IntRange(from = 1900, to = 9999)int yyyy) {


Needs whitespace between ")" and "int".

mernst · 2018-05-05T13:23:48Z

src/main/java/org/jfree/chart/date/SerialDate.java

@@ -538,14 +552,17 @@ public static SerialDate addDays(int days, SerialDate base) {
     *
     * @return a new date.
     */
+    @SuppressWarnings({"index", "value"}) // True positive, fixed upstream bug


Reference an issue tracker bug in upstream JFreeChart repository.
The same comment applies to other comments of this form.

mernst · 2018-05-05T13:25:56Z

src/main/java/org/jfree/chart/date/SerialDate.java

    public static SerialDate addMonths(int months, SerialDate base) {
        int yy = (12 * base.getYYYY() + base.getMonth() + months - 1) / 12;
        if (yy < MINIMUM_YEAR_SUPPORTED || yy > MAXIMUM_YEAR_SUPPORTED) {
            throw new IllegalArgumentException("Call to addMonths resulted in unsupported year");
        }
        int mm = (12 * base.getYYYY() + base.getMonth() + months - 1) % 12 + 1;
-        int dd = Math.min(base.getDayOfMonth(), 
-                SerialDate.lastDayOfMonth(mm, yy));
+        int lastDayOfMonth = SerialDate.lastDayOfMonth(mm, yy);


What is the point of abstracting out this variable?
(Maybe there used to be a @SuppressWarnings on it?)
Please review the diffs and undo any unnecessary changes like this, to minimize the diffs.

mernst · 2018-05-05T13:26:56Z

src/main/java/org/jfree/chart/date/SerialDate.java

@@ -581,7 +599,7 @@ public static SerialDate addYears(int years, SerialDate base) {
     * @return the latest date that falls on the specified day-of-the-week and 
     *         is BEFORE the base date.
     */
-    public static SerialDate getPreviousDayOfWeek(int targetWeekday, 


Don't change whitespace (there was a gratuitous trailing space here).

mernst · 2018-05-08T23:21:03Z

src/main/java/org/jfree/chart/ChartFrame.java

@@ -2,7 +2,7 @@
 * JFreeChart : a free chart library for the Java(tm) platform
 * ===========================================================
 *
- * (C) Copyright 2000-2016, by Object Refinery Limited and Contributors.
+ * (C) Copyright 2000-2018, by Object Refinery Limited and Contributors.


I don't think the copyright dates should be changed.

If this is because you want to revert the JFreeChart source code to an earlier version, that should be done in a separate pull request, to make this one easier to review.

This is different because typetools hasn't merged from upstream, but this branch has (as of about 30 minutes ago when I realized I needed to because the tests were failing!). I certainly didn't change the copyright date. I'll merge upstream into typetools/master to fix the diff.

1. Remove two warning suppressions that aren't required. (One required more annotations.) 2. Added "rewrite" to 14 of the false positives that could be fixed by using Map.Entry.

Plus remove one more.

Work on Guaranteed index false positives.

mernst and others added 30 commits January 27, 2017 14:36

Add check-index Ant target

2ab3356

Ignore generated files

def1f40

Merge github.com:jfree/jfreechart

d64daa4

begin pass at annotations

4fb83b3

annotations

3df94d1

more annotations

0c85993

more annotations

18d524b

more annotations

38a283b

Remove gratuitous diff against upstream

1c4c2bf

Add missing import statements

86154e5

annotations on a bunch of dataset classes

1f24c04

more annotations for 12/13/17

34f6086

mostly finished with data/xy package

bc6dc86

annotations on many packages in org/jfree/data

731f19b

improve annotations on data package. All classes now annotated; reduc…

3dbc048

…ing remaining errors

add suppresswarnings/switch some annotations to remove all? warnings …

56ef4ca

…except those caused by missing annotations in javax.swing classes

finish annotating/suppressing warnings in data package

8d9a536

begin annotating chart super-package

a02a68a

additional annotations in chart packages

ff61de0

fix crash

4b3f6e7

more annotations

7df959f

remove a few more warnings

459770f

more annotations/suppressions

cfdb626

work from plane

61c1649

afternoon work

a04f724

2 warnings! The only remaining type checking warnings are super weird…

bba4571

… and need to be investigated further; everything else has either been removed or suppressed

Ignore output files

8c5de67

updates based on jdk changes

8ae6208

Merge branch 'index' of https://github.com/kelloggm/jfreechart into i…

315c75e

…ndex

change comments

b5947d3

jfree and others added 6 commits February 23, 2018 08:39

Fix for bug jfree#75 - add same outline options as XYLineAndShapeRend…

6eaddd4

…erer.

Update Javadocs for overlays.

4752f0d

Add generics for subtitles list.

893f9b1

Don't use buffering in ChartPanel.

520a4be

fix build

fb40637

remove old lib file, delete them from .gitignore

787248d

kelloggm requested review from mernst and panacekcz May 1, 2018 23:01

panacekcz reviewed May 2, 2018

View reviewed changes

kelloggm added 7 commits May 2, 2018 17:36

minor cosmetic fixes Vlastimil noticed

468caee

add travis integration

2852491

change timeout

90dd716

some whitespace fixes

67929f4

more whitespace fixes

3bf0c0a

minor changes to travis ci config

3d284b2

more whitespace fixes

fc51946

mernst requested changes May 5, 2018

View reviewed changes

kelloggm added 3 commits May 8, 2018 15:52

address a number of Mike's comments

9febaa0

fix merge

7532528

address more of Mike's commments

6fbf1c1

mernst reviewed May 8, 2018

View reviewed changes

kelloggm and others added 9 commits May 8, 2018 17:16

more changes to address Mike's comments

b24c696

add a second extra argument because Maven is bad

2514371

minor fixes

5632a20

Work on Guaranteed index false positives.

4d42a42

1. Remove two warning suppressions that aren't required. (One required more annotations.) 2. Added "rewrite" to 14 of the false positives that could be fixed by using Map.Entry.

Remove warning suppressions that aren't needed.

7db9d8f

Improve comment.

35c8d58

Convert 4 false positives into one.

a88a6cf

Plus remove one more.

Add type argument to iterator and @nonneg

a4fb36d

Merge pull request #2 from smillst/index

0508981

Work on Guaranteed index false positives.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Index Checker annotations #2

Add Index Checker annotations #2

kelloggm commented May 1, 2018 •

edited

Loading

panacekcz left a comment

panacekcz May 2, 2018

kelloggm May 3, 2018

panacekcz May 2, 2018

panacekcz May 2, 2018

panacekcz May 2, 2018

panacekcz May 2, 2018

panacekcz May 2, 2018

panacekcz May 2, 2018

panacekcz May 2, 2018

panacekcz May 2, 2018

panacekcz May 2, 2018

mernst left a comment

mernst May 5, 2018

mernst May 5, 2018

mernst May 5, 2018

mernst May 5, 2018

mernst May 5, 2018

mernst May 5, 2018

mernst May 5, 2018

mernst May 5, 2018

mernst May 5, 2018

mernst May 5, 2018

mernst May 8, 2018

mernst May 8, 2018

kelloggm May 8, 2018

Add Index Checker annotations #2

Are you sure you want to change the base?

Add Index Checker annotations #2

Conversation

kelloggm commented May 1, 2018 • edited Loading

panacekcz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mernst left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kelloggm commented May 1, 2018 •

edited

Loading