:octocat: improve minimum version detection and related tests

src/Common/EccLevel.php

@@ -207,7 +207,18 @@ final class EccLevel{
 	 * @return int[]
 	 */
 	public function getMaxBits():array{
-		return array_column(self::MAX_BITS, $this->getOrdinal());
+		$col = array_column(self::MAX_BITS, $this->getOrdinal());
+
+		unset($col[0]); // remove the inavlid index 0
+
+		return $col;
+	}
+
+	/**
+	 * Returns the maximum bit length for the given version and current ECC level
+	 */
+	public function getMaxBitsForVersion(Version $version):int{
+		return self::MAX_BITS[$version->getVersionNumber()][$this->getOrdinal()];
 	}
 
 }

src/Data/QRData.php

@@ -13,6 +13,7 @@ namespace chillerlan\QRCode\Data;
 use chillerlan\QRCode\Common\{BitBuffer, EccLevel, MaskPattern, Mode, Version};
 use chillerlan\Settings\SettingsContainerInterface;
 
+use function count;
 use function sprintf;
 
 /**
@@ -135,35 +136,48 @@ final class QRData{
 	 * @throws \chillerlan\QRCode\Data\QRCodeDataException
 	 */
 	public function estimateTotalBitLength():int{
-		$length = 4; // 4 bits for the terminator
-		$margin = 0;
+		$length = 0;
 
 		foreach($this->dataSegments as $segment){
-			// data length in bits of the current segment +4 bits for each mode descriptor
-			$length += ($segment->getLengthInBits() + Mode::getLengthBitsForVersion($segment::DATAMODE, 1) + 4);
-
-			if(!$segment instanceof ECI){
-				// mode length bits margin to the next breakpoint
-				$margin += ($segment instanceof Byte) ? 8 : 2;
-			}
-
+			// data length of the current segment
+			$length += $segment->getLengthInBits();
+			// +4 bits for the mode descriptor
+			$length += 4;
+			// Hanzi mode sets an additional 4 bit long subset identifier
 			if($segment instanceof Hanzi){
-				// Hanzi mode sets an additional 4 bit long subset identifier
 				$length += 4;
 			}
+		}
+
+		$provisionalVersion = null;
+
+		foreach($this->maxBitsForEcc as $version => $maxBits){
+
+			if($length <= $maxBits){
+				$provisionalVersion = $version;
+			}
 
 		}
 
-		foreach([9, 26, 40] as $breakpoint){
+		if($provisionalVersion !== null){
 
-			// length bits for the first breakpoint have already been added
-			if($breakpoint > 9){
-				$length += $margin;
+			// add character count indicator bits for the provisional version
+			foreach($this->dataSegments as $segment){
+				$length += Mode::getLengthBitsForVersion($segment::DATAMODE, $provisionalVersion);
 			}
 
-			if($length < $this->maxBitsForEcc[$breakpoint]){
+			// it seems that in some cases the estimated total length is not 100% accurate,
+			// so we substract 4 bits from the total when not in mixed mode
+			if(count($this->dataSegments) <= 1){
+				$length -= 4;
+			}
+
+			// we've got a match!
+			// or let's see if there's a higher version number available
+			if($length <= $this->maxBitsForEcc[$provisionalVersion] || isset($this->maxBitsForEcc[($provisionalVersion + 1)])){
 				return $length;
 			}
+
 		}
 
 		throw new QRCodeDataException(sprintf('estimated data exceeds %d bits', $length));
@@ -199,11 +213,10 @@ final class QRData{
 	 * @throws \chillerlan\QRCode\QRCodeException on data overflow
 	 */
 	private function writeBitBuffer():void{
-		$version  = $this->version->getVersionNumber();
-		$MAX_BITS = $this->maxBitsForEcc[$version];
+		$MAX_BITS = $this->eccLevel->getMaxBitsForVersion($this->version);
 
 		foreach($this->dataSegments as $segment){
-			$segment->write($this->bitBuffer, $version);
+			$segment->write($this->bitBuffer, $this->version->getVersionNumber());
 		}
 
 		// overflow, likely caused due to invalid version setting
@@ -215,7 +228,7 @@ final class QRData{
 
 		// add terminator (ISO/IEC 18004:2000 Table 2)
 		if(($this->bitBuffer->getLength() + 4) <= $MAX_BITS){
-			$this->bitBuffer->put(0, 4);
+			$this->bitBuffer->put(Mode::TERMINATOR, 4);
 		}
 
 		// Padding: ISO/IEC 18004:2000 8.4.9 Bit stream to codeword conversion
@@ -223,6 +236,11 @@ final class QRData{
 		// if the final codeword is not exactly 8 bits in length, it shall be made 8 bits long
 		// by the addition of padding bits with binary value 0
 		while(($this->bitBuffer->getLength() % 8) !== 0){
+
+			if($this->bitBuffer->getLength() === $MAX_BITS){
+				break;
+			}
+
 			$this->bitBuffer->putBit(false);
 		}
 
@@ -231,12 +249,18 @@ final class QRData{
 		// Codewords 11101100 and 00010001 alternately.
 		$alternate = false;
 
-		while($this->bitBuffer->getLength() <= $MAX_BITS){
+		while(($this->bitBuffer->getLength() + 8) <= $MAX_BITS){
 			$this->bitBuffer->put(($alternate) ? 0b00010001 : 0b11101100, 8);
 
 			$alternate = !$alternate;
 		}
 
+		// In certain versions of symbol, it may be necessary to add 3, 4 or 7 Remainder Bits (all zeros)
+		// to the end of the message in order exactly to fill the symbol capacity
+		while($this->bitBuffer->getLength() <= $MAX_BITS){
+			$this->bitBuffer->putBit(false);
+		}
+
 	}
 
 }

tests/Data/AlphaNumTest.php

@@ -17,8 +17,8 @@ use chillerlan\QRCode\Data\AlphaNum;
  */
 final class AlphaNumTest extends DataInterfaceTestAbstract{
 
-	protected string $FQN      = AlphaNum::class;
-	protected string $testdata = '0 $%*+-./:';
+	protected static string $FQN      = AlphaNum::class;
+	protected static string $testdata = '0 $%*+-./:';
 
 	/**
 	 * isAlphaNum() should pass on the 45 defined characters and fail on anything else (e.g. lowercase)

tests/Data/ByteTest.php

@@ -17,8 +17,8 @@ use chillerlan\QRCode\Data\Byte;
  */
 final class ByteTest extends DataInterfaceTestAbstract{
 
-	protected string $FQN      = Byte::class;
-	protected string $testdata = '[¯\_(ツ)_/¯]';
+	protected static string $FQN      = Byte::class;
+	protected static string $testdata = '[¯\_(ツ)_/¯]';
 
 	/**
 	 * isByte() passses any binary string and only fails on empty strings

tests/Data/DataInterfaceTestAbstract.php

@@ -10,23 +10,32 @@
 
 namespace chillerlan\QRCodeTest\Data;
 
-use chillerlan\QRCode\Common\{MaskPattern, Version};
-use chillerlan\QRCode\QROptions;
-use PHPUnit\Framework\TestCase;
+use chillerlan\QRCode\Common\{EccLevel, MaskPattern, Mode, Version};
 use chillerlan\QRCode\Data\{QRCodeDataException, QRData, QRDataModeInterface, QRMatrix};
-use ReflectionClass;
+use chillerlan\QRCode\QROptions;
+use chillerlan\QRCodeTest\QRMaxLengthTrait;
+use Exception,  Generator;
+use PHPUnit\Framework\TestCase;
 
+use function array_map;
 use function hex2bin;
+use function mb_strlen;
+use function mb_substr;
+use function sprintf;
 use function str_repeat;
+use function strlen;
+use function substr;
 
 /**
  * The data interface test abstract
  */
 abstract class DataInterfaceTestAbstract extends TestCase{
+	use QRMaxLengthTrait;
 
-	protected QRData $QRData;
-	protected string $FQN;
-	protected string $testdata;
+	protected QRData              $QRData;
+	protected QRDataModeInterface $dataMode;
+	protected static string       $FQN;
+	protected static string       $testdata;
 
 	protected function setUp():void{
 		$this->QRData = new QRData(new QROptions);
@@ -43,13 +52,10 @@ abstract class DataInterfaceTestAbstract extends TestCase{
 	 * Verifies the QRDataModeInterface instance
 	 */
 	public function testDataModeInstance():void{
-		$datamode = new $this->FQN($this->testdata);
-
-		$this::assertInstanceOf(QRDataModeInterface::class, $datamode);
+		$this::assertInstanceOf(QRDataModeInterface::class, new static::$FQN(static::$testdata));
 	}
 
 	/**
-	 * @see testInitMatrix()
 	 * @return int[][]
 	 */
 	public static function maskPatternProvider():array{
@@ -62,7 +68,7 @@ abstract class DataInterfaceTestAbstract extends TestCase{
 	 * @dataProvider maskPatternProvider
 	 */
 	public function testInitMatrix(int $maskPattern):void{
-		$this->QRData->setData([new $this->FQN($this->testdata)]);
+		$this->QRData->setData([new static::$FQN(static::$testdata)]);
 
 		$matrix = $this->QRData->writeMatrix(new MaskPattern($maskPattern));
 
@@ -70,22 +76,6 @@ abstract class DataInterfaceTestAbstract extends TestCase{
 		$this::assertSame($maskPattern, $matrix->maskPattern()->getPattern());
 	}
 
-	/**
-	 * Tests getting the minimum QR version for the given data
-	 */
-	public function testGetMinimumVersion():void{
-		$this->QRData->setData([new $this->FQN($this->testdata)]);
-
-		$reflection        = new ReflectionClass(QRData::class);
-		$getMinimumVersion = $reflection->getMethod('getMinimumVersion');
-		$getMinimumVersion->setAccessible(true);
-		/** @var \chillerlan\QRCode\Common\Version $version */
-		$version = $getMinimumVersion->invoke($this->QRData);
-
-		$this::assertInstanceOf(Version::class, $version);
-		$this::assertSame(1, $version->getVersionNumber());
-	}
-
 	abstract public static function stringValidateProvider():array;
 
 	/**
@@ -95,7 +85,7 @@ abstract class DataInterfaceTestAbstract extends TestCase{
 	 */
 	public function testValidateString(string $string, bool $expected):void{
 		/** @noinspection PhpUndefinedMethodInspection */
-		$this::assertSame($expected, $this->FQN::validateString($string));
+		$this::assertSame($expected, static::$FQN::validateString($string));
 	}
 
 	/**
@@ -105,7 +95,7 @@ abstract class DataInterfaceTestAbstract extends TestCase{
 	 */
 	public function testBinaryStringInvalid():void{
 		/** @noinspection PhpUndefinedMethodInspection */
-		$this::assertFalse($this->FQN::validateString(hex2bin('01015989f47dff8e852122117e04c90b9f15defc1c36477b1fe1')));
+		$this::assertFalse(static::$FQN::validateString(hex2bin('01015989f47dff8e852122117e04c90b9f15defc1c36477b1fe1')));
 	}
 
 	/**
@@ -121,21 +111,114 @@ abstract class DataInterfaceTestAbstract extends TestCase{
 	 * @dataProvider versionBreakpointProvider
 	 */
 	public function testDecodeSegment(int $version):void{
-		$options = new QROptions;
+		$options          = new QROptions;
 		$options->version = $version;
 
 		// invoke a datamode interface
-		/** @var \chillerlan\QRCode\Data\QRDataModeInterface $datamodeInterface */
-		$datamodeInterface = new $this->FQN($this->testdata);
+		$this->dataMode = new static::$FQN(static::$testdata);
 		// invoke a QRData instance and write data
-		$this->QRData = new QRData($options, [$datamodeInterface]);
+		$this->QRData = new QRData($options, [$this->dataMode]);
 		// get the filled bitbuffer
 		$bitBuffer = $this->QRData->getBitBuffer();
 		// read the first 4 bits
-		$this::assertSame($datamodeInterface::DATAMODE, $bitBuffer->read(4));
+		$this::assertSame($this->dataMode::DATAMODE, $bitBuffer->read(4));
 		// decode the data
-		/** @noinspection PhpUndefinedMethodInspection */
-		$this::assertSame($this->testdata, $this->FQN::decodeSegment($bitBuffer, $options->version));
+		$this::assertSame(static::$testdata, $this->dataMode::decodeSegment($bitBuffer, $options->version));
+	}
+
+	/**
+	 * Generates test data for each data mode:
+	 *
+	 *   - version
+	 *   - ECC level
+	 *   - a string that contains the maximum amount of characters for the given mode
+	 *   - a string that contains characters for the given mode and that exceeds the maximum length by one/two character(s)
+	 *   - the maximum allowed character length
+	 *
+	 * @throws \Exception
+	 */
+	public static function maxLengthProvider():Generator{
+		$eccLevels = array_map(fn(int $ecc):EccLevel => new EccLevel($ecc), [EccLevel::L, EccLevel::M, EccLevel::Q, EccLevel::H]);
+		$str       = str_repeat(static::$testdata, 1000);
+		$mb        = (static::$FQN::DATAMODE === Mode::KANJI || static::$FQN::DATAMODE === Mode::HANZI);
+
+		for($v = 1; $v <= 40; $v++){
+			$version = new Version($v);
+
+			foreach($eccLevels as $eccLevel){
+				// maximum characters per ecc/mode
+				$len  = static::getMaxLengthForMode(static::$FQN::DATAMODE, $version, $eccLevel);
+				// a string that contains the maximum amount of characters for the given mode
+				$val  = ($mb) ? mb_substr($str, 0, $len) : substr($str, 0, $len);
+				// same as above but character count exceeds
+				// kanji/hanzi may have space for a single character, so we add 2 to make sure we exceed
+				$val1 = ($mb) ? mb_substr($str, 0, ($len + 2)) : substr($str, 0, ($len + 1));
+				// array key
+				$key  = sprintf('version: %s%s (%s)', $version, $eccLevel, $len);
+
+				if((($mb) ? mb_strlen($val) : strlen($val)) !== $len){
+					throw new Exception('output length does not match');
+				}
+
+				yield $key => [$version, $eccLevel, $val, $val1, $len];
+			}
+		}
+
+	}
+
+	/**
+	 * @dataProvider maxLengthProvider
+	 */
+	public function testMaxLength(Version $version, EccLevel $eccLevel, string $str):void{
+		$options           = new QROptions;
+		$options->version  = $version->getVersionNumber();
+		$options->eccLevel = $eccLevel->getLevel();
+		$this->dataMode    = new static::$FQN($str);
+		$this->QRData      = new QRData($options, [$this->dataMode]);
+		$bitBuffer         = $this->QRData->getBitBuffer();
+
+		$this::assertSame($this->dataMode::DATAMODE, $bitBuffer->read(4));
+		$this::assertSame($str, $this->dataMode::decodeSegment($bitBuffer, $options->version));
+	}
+
+	/**
+	 * Tests getting the minimum QR version for the given data
+	 *
+	 * @dataProvider maxLengthProvider
+	 */
+	public function testGetMinimumVersion(Version $version, EccLevel $eccLevel, string $str):void{
+		$options           = new QROptions;
+		$options->version  = Version::AUTO;
+		$options->eccLevel = $eccLevel->getLevel();
+		$this->dataMode    = new static::$FQN($str);
+		$this->QRData      = new QRData($options, [$this->dataMode]);
+		$bitBuffer         = $this->QRData->getBitBuffer();
+
+		$this::assertLessThanOrEqual($eccLevel->getMaxBitsForVersion($version), $this->QRData->estimateTotalBitLength());
+
+		$minimumVersionNumber = $this->QRData->getMinimumVersion()->getVersionNumber();
+
+		$this::assertSame($version->getVersionNumber(), $minimumVersionNumber);
+		// verify the encoded data
+		$this::assertSame($this->dataMode::DATAMODE, $bitBuffer->read(4));
+		$this::assertSame($str, $this->dataMode::decodeSegment($bitBuffer, $minimumVersionNumber));
+	}
+
+	/**
+	 * Tests if an exception is thrown on data overflow
+	 *
+	 * @dataProvider maxLengthProvider
+	 */
+	public function testMaxLengthOverflowException(Version $version, EccLevel $eccLevel, string $str, string $str1):void{
+		$this->expectException(QRCodeDataException::class);
+		$this->expectExceptionMessage('code length overflow');
+
+		$options           = new QROptions;
+		$options->version  = $version->getVersionNumber();
+		$options->eccLevel = $eccLevel->getLevel();
+
+		/** @phan-suppress-next-line PhanNoopNew */
+		new QRData($options, [new static::$FQN($str1)]);
 	}
 
 	/**
@@ -145,20 +228,7 @@ abstract class DataInterfaceTestAbstract extends TestCase{
 		$this->expectException(QRCodeDataException::class);
 		$this->expectExceptionMessage('data exceeds');
 
-		$this->QRData->setData([new $this->FQN(str_repeat($this->testdata, 1337))]);
-	}
-
-	/**
-	 * Tests if an exception is thrown on data overflow
-	 */
-	public function testCodeLengthOverflowException():void{
-		$this->expectException(QRCodeDataException::class);
-		$this->expectExceptionMessage('code length overflow');
-
-		$this->QRData = new QRData(
-			new QROptions(['version' => 4]),
-			[new $this->FQN(str_repeat($this->testdata, 1337))]
-		);
+		$this->QRData->setData([new static::$FQN(str_repeat(static::$testdata, 1337))]);
 	}
 
 	/**
@@ -169,7 +239,7 @@ abstract class DataInterfaceTestAbstract extends TestCase{
 		$this->expectExceptionMessage('invalid data');
 
 		/** @phan-suppress-next-line PhanNoopNew */
-		new $this->FQN('##');
+		new static::$FQN('##');
 	}
 
 	/**
@@ -180,7 +250,7 @@ abstract class DataInterfaceTestAbstract extends TestCase{
 		$this->expectExceptionMessage('invalid data');
 
 		/** @phan-suppress-next-line PhanNoopNew */
-		new $this->FQN('');
+		new static::$FQN('');
 	}
 
 }

tests/Data/HanziTest.php

@@ -19,8 +19,8 @@ use function bin2hex, chr, defined, sprintf;
  */
 final class HanziTest extends DataInterfaceTestAbstract{
 
-	protected string $FQN      = Hanzi::class;
-	protected string $testdata = '无可奈何燃花作香';
+	protected static string $FQN      = Hanzi::class;
+	protected static string $testdata = '无可奈何燃花作香';
 
 	/**
 	 * isGB2312() should pass on Hanzi/GB2312 characters and fail on everything else

tests/Data/KanjiTest.php

@@ -19,8 +19,8 @@ use function bin2hex, chr, defined, sprintf;
  */
 final class KanjiTest extends DataInterfaceTestAbstract{
 
-	protected string $FQN      = Kanji::class;
-	protected string $testdata = '漂う花の香り';
+	protected static string $FQN      = Kanji::class;
+	protected static string $testdata = '漂う花の香り';
 
 	/**
 	 * isKanji() should pass on Kanji/SJIS characters and fail on everything else

tests/Data/NumberTest.php

@@ -17,8 +17,8 @@ use chillerlan\QRCode\Data\Number;
  */
 final class NumberTest extends DataInterfaceTestAbstract{
 
-	protected string $FQN      = Number::class;
-	protected string $testdata = '0123456789';
+	protected static string $FQN      = Number::class;
+	protected static string $testdata = '0123456789';
 
 	/**
 	 * isNumber() should pass on any number and fail on anything else

tests/QRMaxLengthTrait.php

@@ -78,21 +78,40 @@ trait QRMaxLengthTrait{
 	 * @throws \chillerlan\QRCode\QRCodeException
 	 * @codeCoverageIgnore
 	 */
-	public static function getMaxLengthForMode(int $mode, Version $version, EccLevel $eccLevel):?int{
+	public static function getMaxLengthForMode(int $mode, Version $version, EccLevel $eccLevel):int{
 
-		$dataModes = [
+		$dataMode = ([
 			Mode::NUMBER   => 0,
 			Mode::ALPHANUM => 1,
 			Mode::BYTE     => 2,
 			Mode::KANJI    => 3,
 			Mode::HANZI    => 3, // similar to kanji mode
-		];
+		][$mode] ?? false);
 
-		if(!isset($dataModes[$mode])){
+		if($dataMode === false){
 			throw new QRCodeException('invalid $mode');
 		}
+
+		$ver = $version->getVersionNumber();
+		$ecc = $eccLevel->getOrdinal();
+
+		if(!isset(static::$MAX_LENGTH[$ver])){
+			throw new QRCodeException('invalid $version');
+		}
+
+		if(!isset(static::$MAX_LENGTH[$ver][$dataMode][$ecc])){
+			throw new QRCodeException('invalid $ecc');
+		}
+
 		/** @SuppressWarnings(PHPMD.UndefinedVariable) */
-		return (static::$MAX_LENGTH[$version->getVersionNumber()][$dataModes[$mode]][$eccLevel->getOrdinal()] ?? null);
+		$maxlength = static::$MAX_LENGTH[$ver][$dataMode][$ecc];
+
+		// Hanzi mode sets an additional 4 bit long subset identifier
+		if($mode === Mode::HANZI){
+			return ($maxlength - 1);
+		}
+
+		return $maxlength;
 	}
 
 }