#64350 - Sonar fix - "Iterator.next()" methods should throw "NoSuchElementException"

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1876525 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java

@@ -18,6 +18,7 @@
 package org.apache.poi.hssf.record.aggregates;
 
 import java.util.Iterator;
+import java.util.NoSuchElementException;
 
 import org.apache.poi.hssf.model.RecordStream;
 import org.apache.poi.hssf.record.BlankRecord;
@@ -338,8 +339,9 @@ public final class ValueRecordsAggregate implements Iterable<CellValueRecordInte
 		}
 
 		public CellValueRecordInterface next() {
-			if (!hasNext())
+			if (!hasNext()) {
-				throw new IndexOutOfBoundsException("iterator has no next");
+				throw new NoSuchElementException();
+			}
 
 			curRowIndex = nextRowIndex;
 			curColIndex = nextColIndex;

src/java/org/apache/poi/poifs/filesystem/FilteringDirectoryNode.java

@@ -29,6 +29,7 @@ import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.NoSuchElementException;
 import java.util.Set;
 
 import org.apache.poi.hpsf.ClassID;
@@ -213,6 +214,10 @@ public class FilteringDirectoryNode implements DirectoryEntry
       }
 
       public Entry next() {
+         if (!hasNext()) {
+            throw new NoSuchElementException();
+         }
+
          Entry e = next;
          locateNext();
          return e;

src/java/org/apache/poi/poifs/filesystem/POIFSMiniStore.java

@@ -23,6 +23,7 @@ import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.util.Iterator;
 import java.util.List;
+import java.util.NoSuchElementException;
 
 import org.apache.poi.poifs.common.POIFSConstants;
 import org.apache.poi.poifs.property.RootProperty;
@@ -68,9 +69,7 @@ public class POIFSMiniStore extends BlockStore
           it.next();
        }
        ByteBuffer dataBlock = it.next();
-       if(dataBlock == null) {
+       assert(dataBlock != null);
-          throw new IndexOutOfBoundsException("Big block " + bigBlockNumber + " outside stream");
-       }
 
        // Position ourselves, and take a slice
        dataBlock.position(
@@ -94,7 +93,7 @@ public class POIFSMiniStore extends BlockStore
        if (! firstInStore) {
            try {
               return getBlockAt(offset);
-           } catch(IndexOutOfBoundsException e) {}
+           } catch(NoSuchElementException e) {}
        }
 
        // Need to extend the stream

src/java/org/apache/poi/poifs/filesystem/POIFSStream.java

@@ -23,6 +23,7 @@ import java.io.IOException;
 import java.io.OutputStream;
 import java.nio.ByteBuffer;
 import java.util.Iterator;
+import java.util.NoSuchElementException;
 
 import org.apache.poi.poifs.common.POIFSConstants;
 import org.apache.poi.poifs.filesystem.BlockStore.ChainLoopDetector;
@@ -157,8 +158,8 @@ public class POIFSStream implements Iterable<ByteBuffer>
       }
 
       public ByteBuffer next() {
-         if(nextBlock == POIFSConstants.END_OF_CHAIN) {
+         if (!hasNext()) {
-            throw new IndexOutOfBoundsException("Can't read past the end of the stream");
+            throw new NoSuchElementException("Can't read past the end of the stream");
          }
 
          try {

src/java/org/apache/poi/util/StringCodepointsIterable.java

@@ -18,6 +18,7 @@
 package org.apache.poi.util;
 
 import java.util.Iterator;
+import java.util.NoSuchElementException;
 
 // based on https://gist.github.com/EmmanuelOga/48df70b27ead4d80234b
 @Internal
@@ -37,6 +38,9 @@ public class StringCodepointsIterable implements Iterable<String> {
 
         @Override
         public String next() {
+            if (!hasNext()) {
+                throw new NoSuchElementException();
+            }
             int codePoint = StringCodepointsIterable.this.string.codePointAt(index);
             index += Character.charCount(codePoint);
             return new String(Character.toChars(codePoint));

src/java/org/apache/poi/util/StringUtil.java

@@ -17,9 +17,10 @@
 
 package org.apache.poi.util;
 
+import static java.nio.charset.StandardCharsets.ISO_8859_1;
+
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
-import java.util.Iterator;
 import java.util.Locale;
 
 /**
@@ -27,7 +28,6 @@ import java.util.Locale;
  */
 @Internal
 public final class StringUtil {
-    private static final Charset ISO_8859_1 = StandardCharsets.ISO_8859_1;
     //arbitrarily selected; may need to increase
     private static final int MAX_RECORD_LENGTH = 10000000;
 
@@ -305,38 +305,6 @@ public final class StringUtil {
         return haystack.regionMatches(true, start, suffix, 0, length);
     }
 
-    /**
-     * An Iterator over an array of Strings.
-     */
-    public static class StringsIterator implements Iterator<String> {
-        private String[] strings = {};
-        private int position;
-
-        public StringsIterator(String[] strings) {
-            if (strings != null) {
-                this.strings = strings.clone();
-            }
-        }
-
-        @Override
-        public boolean hasNext() {
-            return position < strings.length;
-        }
-
-        @Override
-        public String next() {
-            int ourPos = position++;
-            if (ourPos >= strings.length) {
-                throw new ArrayIndexOutOfBoundsException(ourPos);
-            }
-            return strings[ourPos];
-        }
-
-        @Override
-        public void remove() {
-        }
-    }
-
     @Internal
     public static String toLowerCase(char c) {
         return Character.toString(c).toLowerCase(Locale.ROOT);

src/scratchpad/src/org/apache/poi/hemf/record/emf/HemfComment.java

@@ -25,6 +25,7 @@ import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.NoSuchElementException;
 import java.util.function.Supplier;
 
 import org.apache.poi.common.usermodel.GenericRecord;
@@ -172,6 +173,9 @@ public class HemfComment {
 
         @Override
         public EmfCommentData next() {
+            if (!hasNext()) {
+                throw new NoSuchElementException();
+            }
             EmfCommentData toReturn = currentRecord;
             final boolean isEOF = (limit == -1 || leis.getReadIndex() >= startIdx+limit);
             // (currentRecord instanceof HemfPlusMisc.EmfEof)

src/scratchpad/src/org/apache/poi/hemf/record/emf/HemfRecordIterator.java

@@ -19,6 +19,7 @@ package org.apache.poi.hemf.record.emf;
 
 import java.io.IOException;
 import java.util.Iterator;
+import java.util.NoSuchElementException;
 
 import org.apache.poi.util.LittleEndianConsts;
 import org.apache.poi.util.LittleEndianInputStream;
@@ -44,6 +45,9 @@ public class HemfRecordIterator implements Iterator<HemfRecord> {
 
     @Override
     public HemfRecord next() {
+        if (!hasNext()) {
+            throw new NoSuchElementException();
+        }
         HemfRecord toReturn = currentRecord;
         currentRecord = (currentRecord instanceof HemfMisc.EmfEof) ? null : _next();
         return toReturn;

src/scratchpad/src/org/apache/poi/hemf/record/emfplus/HemfPlusRecordIterator.java

@@ -19,6 +19,7 @@ package org.apache.poi.hemf.record.emfplus;
 
 import java.io.IOException;
 import java.util.Iterator;
+import java.util.NoSuchElementException;
 
 import org.apache.poi.util.LittleEndianInputStream;
 import org.apache.poi.util.RecordFormatException;
@@ -49,6 +50,9 @@ public class HemfPlusRecordIterator implements Iterator<HemfPlusRecord> {
 
     @Override
     public HemfPlusRecord next() {
+        if (!hasNext()) {
+            throw new NoSuchElementException();
+        }
         HemfPlusRecord toReturn = currentRecord;
         // add the size for recordId/flags/recordSize/dataSize = 12 bytes
         final boolean isEOF = (limit == -1 || (leis.getReadIndex()-startIdx)+12 > limit);

src/scratchpad/src/org/apache/poi/hsmf/extractor/OutlookTextExtractor.java

@@ -16,6 +16,17 @@
 ==================================================================== */
 package org.apache.poi.hsmf.extractor;
 
+import static org.apache.poi.util.StringUtil.startsWithIgnoreCase;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.text.SimpleDateFormat;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.Locale;
+
 import org.apache.poi.extractor.POIOLE2TextExtractor;
 import org.apache.poi.hsmf.MAPIMessage;
 import org.apache.poi.hsmf.datatypes.AttachmentChunks;
@@ -24,16 +35,6 @@ import org.apache.poi.hsmf.exceptions.ChunkNotFoundException;
 import org.apache.poi.poifs.filesystem.DirectoryNode;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
 import org.apache.poi.util.LocaleUtil;
-import org.apache.poi.util.Removal;
-import org.apache.poi.util.StringUtil.StringsIterator;
-
-import java.io.File;
-import java.io.IOException;
-import java.io.InputStream;
-import java.text.SimpleDateFormat;
-import java.util.Locale;
-
-import static org.apache.poi.util.StringUtil.startsWithIgnoreCase;
 
 /**
  * A text extractor for HSMF (Outlook) .msg files.
@@ -90,13 +91,11 @@ public class OutlookTextExtractor extends POIOLE2TextExtractor {
         msg.guess7BitEncoding();
 
         // Off we go
-        StringsIterator emails;
+        Iterator<String> emails;
         try {
-            emails = new StringsIterator(
+            emails = Arrays.asList(msg.getRecipientEmailAddressList()).iterator();
-                    msg.getRecipientEmailAddressList()
-            );
         } catch (ChunkNotFoundException e) {
-            emails = new StringsIterator(new String[0]);
+            emails = Collections.emptyIterator();
         }
 
         try {
@@ -174,7 +173,7 @@ public class OutlookTextExtractor extends POIOLE2TextExtractor {
      * of emails, and does its best to return something like
      * "Nick <nick@example.com>; Jim <jim@example.com>"
      */
-    protected void handleEmails(StringBuilder s, String type, String displayText, StringsIterator emails) {
+    protected void handleEmails(StringBuilder s, String type, String displayText, Iterator<String> emails) {
         if (displayText == null || displayText.length() == 0) {
             return;
         }

src/testcases/org/apache/poi/poifs/filesystem/TestFilteringDirectoryNode.java

@@ -18,18 +18,23 @@
 
 package org.apache.poi.poifs.filesystem;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
 import java.io.ByteArrayInputStream;
 import java.io.FileNotFoundException;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.NoSuchElementException;
 
 import org.junit.Before;
 import org.junit.Test;
 
-import static org.junit.Assert.*;
-
 /**
  * Class to test FilteringDirectoryNode functionality
  */
@@ -72,12 +77,16 @@ public final class TestFilteringDirectoryNode {
        assertEquals(dirA, i.next());
        assertEquals(dirB, i.next());
        assertEquals(eRoot, i.next());
+       try {
            assertNull(i.next());
+           fail("Should throw NoSuchElementException when depleted");
+       } catch (NoSuchElementException ignored) {
+       }
    }
 
    @Test
    public void testChildFiltering() throws Exception {
-      List<String> excl = Arrays.asList(new String[]{"NotThere", "AlsoNotThere", eRoot.getName()});
+      List<String> excl = Arrays.asList("NotThere", "AlsoNotThere", eRoot.getName());
       FilteringDirectoryNode d = new FilteringDirectoryNode(fs.getRoot(), excl);
 
       assertEquals(2, d.getEntryCount());
@@ -96,11 +105,15 @@ public final class TestFilteringDirectoryNode {
       Iterator<Entry> i = d.getEntries();
       assertEquals(dirA, i.next());
       assertEquals(dirB, i.next());
+      try {
           assertNull(i.next());
+          fail("Should throw NoSuchElementException when depleted");
+      } catch (NoSuchElementException ignored) {
+      }
 
 
       // Filter more
-      excl = Arrays.asList(new String[]{"NotThere", "AlsoNotThere", eRoot.getName(), dirA.getName()});
+      excl = Arrays.asList("NotThere", "AlsoNotThere", eRoot.getName(), dirA.getName());
       d = new FilteringDirectoryNode(fs.getRoot(), excl);
 
       assertEquals(1, d.getEntryCount());
@@ -122,11 +135,15 @@ public final class TestFilteringDirectoryNode {
 
       i = d.getEntries();
       assertEquals(dirB, i.next());
+       try {
            assertNull(i.next());
+           fail("Should throw NoSuchElementException when depleted");
+       } catch (NoSuchElementException ignored) {
+       }
 
 
       // Filter everything
-      excl = Arrays.asList(new String[]{"NotThere", eRoot.getName(), dirA.getName(), dirB.getName()});
+      excl = Arrays.asList("NotThere", eRoot.getName(), dirA.getName(), dirB.getName());
       d = new FilteringDirectoryNode(fs.getRoot(), excl);
 
       assertEquals(0, d.getEntryCount());
@@ -151,17 +168,19 @@ public final class TestFilteringDirectoryNode {
       }
 
       i = d.getEntries();
+       try {
            assertNull(i.next());
+           fail("Should throw NoSuchElementException when depleted");
+       } catch (NoSuchElementException ignored) {
+       }
    }
 
    @Test
    public void testNestedFiltering() throws Exception {
-      List<String> excl = Arrays.asList(new String[]{
+      List<String> excl = Arrays.asList(dirA.getName() + "/" + "MadeUp",
-              dirA.getName() + "/" + "MadeUp",
                                         dirA.getName() + "/" + eA.getName(),
                                         dirA.getName() + "/" + dirAA.getName() + "/Test",
-              eRoot.getName()
+                                        eRoot.getName());
-      });
       FilteringDirectoryNode d = new FilteringDirectoryNode(fs.getRoot(), excl);
 
       // Check main

src/testcases/org/apache/poi/poifs/filesystem/TestPOIFSMiniStore.java

@@ -26,6 +26,7 @@ import static org.junit.Assert.fail;
 import java.io.ByteArrayInputStream;
 import java.nio.ByteBuffer;
 import java.util.Iterator;
+import java.util.NoSuchElementException;
 
 import org.apache.poi.POIDataSamples;
 import org.apache.poi.poifs.common.POIFSConstants;
@@ -289,7 +290,7 @@ public final class TestPOIFSMiniStore {
       try {
          ministore.getBlockAt(184);
          fail("No block at 184");
-      } catch(IndexOutOfBoundsException e) {}
+      } catch(NoSuchElementException e) {}
 
       // The ministore itself is made up of 23 big blocks
       Iterator<ByteBuffer> it = new POIFSStream(fs, fs.getRoot().getProperty().getStartBlock()).getBlockIterator();
@@ -319,7 +320,7 @@ public final class TestPOIFSMiniStore {
       try {
          ministore.getBlockAt(192);
          fail("No block at 192");
-      } catch(IndexOutOfBoundsException e) {}
+      } catch(NoSuchElementException e) {}
 
 
       // Now try writing through to 192, check that the SBAT and blocks are there

src/testcases/org/apache/poi/poifs/filesystem/TestPOIFSStream.java

@@ -36,6 +36,7 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.ByteBuffer;
 import java.util.Iterator;
+import java.util.NoSuchElementException;
 
 import org.apache.poi.POIDataSamples;
 import org.apache.poi.hpsf.DocumentSummaryInformation;
@@ -72,12 +73,8 @@ public final class TestPOIFSStream {
       POIFSStream stream = new POIFSStream(fs, 98);
       Iterator<ByteBuffer> i = stream.getBlockIterator();
       assertTrue(i.hasNext());
-      assertTrue(i.hasNext());
-      assertTrue(i.hasNext());
       ByteBuffer b = i.next();
       assertFalse(i.hasNext());
-      assertFalse(i.hasNext());
-      assertFalse(i.hasNext());
 
       // Check the contents
       assertEquals((byte)0x81, b.get());
@@ -103,15 +100,10 @@ public final class TestPOIFSStream {
       POIFSStream stream = new POIFSStream(fs, 97);
       Iterator<ByteBuffer> i = stream.getBlockIterator();
       assertTrue(i.hasNext());
-      assertTrue(i.hasNext());
-      assertTrue(i.hasNext());
       ByteBuffer b97 = i.next();
       assertTrue(i.hasNext());
-      assertTrue(i.hasNext());
       ByteBuffer b98 = i.next();
       assertFalse(i.hasNext());
-      assertFalse(i.hasNext());
-      assertFalse(i.hasNext());
 
       // Check the contents of the 1st block
       assertEquals((byte)0x01, b97.get());
@@ -169,18 +161,21 @@ public final class TestPOIFSStream {
 
       // Check the contents
       //  1st block is at 0
+      assertNotNull(b0);
       assertEquals((byte)0x9e, b0.get());
       assertEquals((byte)0x75, b0.get());
       assertEquals((byte)0x97, b0.get());
       assertEquals((byte)0xf6, b0.get());
 
       //  2nd block is at 1
+      assertNotNull(b1);
       assertEquals((byte)0x86, b1.get());
       assertEquals((byte)0x09, b1.get());
       assertEquals((byte)0x22, b1.get());
       assertEquals((byte)0xfb, b1.get());
 
       //  last block is at 89
+      assertNotNull(b22);
       assertEquals((byte)0xfe, b22.get());
       assertEquals((byte)0xff, b22.get());
       assertEquals((byte)0x00, b22.get());
@@ -204,18 +199,12 @@ public final class TestPOIFSStream {
       POIFSStream stream = new POIFSStream(fs, 0);
       Iterator<ByteBuffer> i = stream.getBlockIterator();
       assertTrue(i.hasNext());
-      assertTrue(i.hasNext());
-      assertTrue(i.hasNext());
       ByteBuffer b0 = i.next();
       assertTrue(i.hasNext());
-      assertTrue(i.hasNext());
       ByteBuffer b1 = i.next();
       assertTrue(i.hasNext());
-      assertTrue(i.hasNext());
       ByteBuffer b2 = i.next();
       assertFalse(i.hasNext());
-      assertFalse(i.hasNext());
-      assertFalse(i.hasNext());
 
       // Check the contents of the 1st block
       assertEquals((byte)0x9E, b0.get());
@@ -304,17 +293,12 @@ public final class TestPOIFSStream {
       POIFSStream stream = new POIFSStream(ministore, 178);
       Iterator<ByteBuffer> i = stream.getBlockIterator();
       assertTrue(i.hasNext());
-      assertTrue(i.hasNext());
-      assertTrue(i.hasNext());
       ByteBuffer b178 = i.next();
       assertTrue(i.hasNext());
-      assertTrue(i.hasNext());
       ByteBuffer b179 = i.next();
       assertTrue(i.hasNext());
       ByteBuffer b180 = i.next();
       assertFalse(i.hasNext());
-      assertFalse(i.hasNext());
-      assertFalse(i.hasNext());
 
       // Check the contents of the 1st block
       assertEquals((byte)0xfe, b178.get());
@@ -803,7 +787,7 @@ public final class TestPOIFSStream {
           try {
               ministore.getBlockAt(184);
               fail("Block 184 should be off the end of the list");
-          } catch (IndexOutOfBoundsException e) {
+          } catch (NoSuchElementException e) {
           }
 
           data = new byte[64 * 6 + 12];

src/testcases/org/apache/poi/util/TestStringUtil.java

@@ -24,7 +24,6 @@ import static org.junit.Assert.fail;
 
 import java.nio.charset.Charset;
 
-import org.apache.poi.util.StringUtil.StringsIterator;
 import org.junit.Test;
 
 /**
@@ -127,52 +126,6 @@ public class TestStringUtil {
         }
     }
 
-    @Test
-    public void testStringsIterator() {
-       StringsIterator i;
-
-       
-       i = new StringsIterator(new String[0]);
-       assertFalse(i.hasNext());
-       try {
-          i.next();
-          fail();
-       } catch(ArrayIndexOutOfBoundsException e) {
-           // expected here
-       }
-
-       
-       i = new StringsIterator(new String[] {"1"});
-       assertTrue(i.hasNext());
-       assertEquals("1", i.next());
-       
-       assertFalse(i.hasNext());
-       try {
-          i.next();
-          fail();
-       } catch(ArrayIndexOutOfBoundsException e) {
-           // expected here
-       }
-
-       
-       i = new StringsIterator(new String[] {"1","2","3"});
-       assertTrue(i.hasNext());
-       assertEquals("1", i.next());
-       assertTrue(i.hasNext());
-       assertEquals("2", i.next());
-       assertTrue(i.hasNext());
-       assertEquals("3", i.next());
-       
-       assertFalse(i.hasNext());
-       try {
-          i.next();
-          fail();
-       } catch(ArrayIndexOutOfBoundsException e) {
-           // expected here
-       }
-    }
-    
-
     @Test
     public void startsWithIgnoreCase() {
         assertTrue("same string", StringUtil.startsWithIgnoreCase("Apache POI", "Apache POI"));