Use configy library to parse dub.json file

Geod24 · Geod24 · commit a8aa2341be84 · 2022-08-28T12:30:56.000+09:00
This will provide a vastly superior UX to the users:
- They will be warned about typos;
- The type information provided will be more accurate and always include file/line infos
  (e.g. no more 'Invalid SemVer versions');
- We can expose a better recipe struct to library users on the long run,
  by separating the actual data DUB uses from what it reads;
- We can be more descriptive about errors / intent, for example by using `SetInfo`;
diff --git a/source/dub/commandline.d b/source/dub/commandline.d
@@ -286,7 +286,7 @@ struct CommandLineHandler
 
 		// make the CWD package available so that for example sub packages can reference their
 		// parent package.
-		try dub.packageManager.getOrLoadPackage(NativePath(options.root_path));
+		try dub.packageManager.getOrLoadPackage(NativePath(options.root_path), NativePath.init, false, StrictMode.Warn);
 		catch (Exception e) { logDiagnostic("No valid package found in current working directory: %s", e.msg); }
 
 		return dub;
diff --git a/source/dub/internal/utils.d b/source/dub/internal/utils.d
@@ -20,6 +20,7 @@ import std.array : appender, array;
 import std.conv : to;
 import std.exception : enforce;
 import std.file;
+import std.format;
 import std.string : format;
 import std.process;
 import std.traits : isIntegral;
@@ -689,3 +690,53 @@ string getModuleNameFromFile(string filePath) {
 	logDiagnostic("Get module name from path: %s", filePath);
 	return getModuleNameFromContent(fileContent);
 }
+
+/**
+ * Compare two instances of the same type for equality,
+ * providing a rich error message on failure.
+ *
+ * This function will recurse into composite types (struct, AA, arrays)
+ * and compare element / member wise, taking opEquals into account,
+ * to provide the most accurate reason why comparison failed.
+ */
+void deepCompare (T) (
+	in T result, in T expected, string file = __FILE__, size_t line = __LINE__)
+{
+	deepCompareImpl!T(result, expected, T.stringof, file, line);
+}
+
+void deepCompareImpl (T) (
+	in T result, in T expected, string path, string file, size_t line)
+{
+	static if (is(T == struct) && !is(typeof(T.init.opEquals(T.init)) : bool))
+	{
+		static foreach (idx; 0 .. T.tupleof.length)
+			deepCompareImpl(result.tupleof[idx], expected.tupleof[idx],
+							format("%s.%s", path, __traits(identifier, T.tupleof[idx])),
+							file, line);
+	}
+	else static if (is(T : KeyT[ValueT], KeyT, ValueT))
+	{
+		if (result.length != expected.length)
+			throw new Exception(
+				format("%s: AA has different number of entries (%s != %s): %s != %s",
+					   path, result.length, expected.length, result, expected),
+				file, line);
+		foreach (key, value; expected)
+		{
+			if (auto ptr = key in result)
+				deepCompareImpl(*ptr, value, format("%s[%s]", path, key), file, line);
+			else
+				throw new Exception(
+					format("Expected key %s[%s] not present in result. %s != %s",
+						   path, key, result, expected), file, line);
+		}
+	}
+	else if (result != expected) {
+		static if (is(T == struct) && is(typeof(T.init.opEquals(T.init)) : bool))
+			path ~= ".opEquals";
+		throw new Exception(
+			format("%s: result != expected: %s != %s", path, result, expected),
+			file, line);
+	}
+}
diff --git a/source/dub/package_.d b/source/dub/package_.d
@@ -21,6 +21,8 @@ import dub.internal.vibecompat.core.file;
 import dub.internal.vibecompat.data.json;
 import dub.internal.vibecompat.inet.path;
 
+import configy.Read : StrictMode;
+
 import std.algorithm;
 import std.array;
 import std.conv;
@@ -169,8 +171,11 @@ class Package {
 			version_override = Optional version to associate to the package
 				instead of the one declared in the package recipe, or the one
 				determined by invoking the VCS (GIT currently).
+			mode = Whether to issue errors, warning, or ignore unknown keys in dub.json
 	*/
-	static Package load(NativePath root, NativePath recipe_file = NativePath.init, Package parent = null, string version_override = "")
+	static Package load(NativePath root, NativePath recipe_file = NativePath.init,
+		Package parent = null, string version_override = "",
+		StrictMode mode = StrictMode.Ignore)
 	{
 		import dub.recipe.io;
 
@@ -181,7 +186,7 @@ class Package {
 				.format(root.toNativeString(),
 					packageInfoFiles.map!(f => cast(string)f.filename).join("/")));
 
-		auto recipe = readPackageRecipe(recipe_file, parent ? parent.name : null);
+		auto recipe = readPackageRecipe(recipe_file, parent ? parent.name : null, mode);
 
 		auto ret = new Package(recipe, root, parent, version_override);
 		ret.m_infoFile = recipe_file;
diff --git a/source/dub/packagemanager.d b/source/dub/packagemanager.d
@@ -14,6 +14,9 @@ import dub.internal.vibecompat.data.json;
 import dub.internal.vibecompat.inet.path;
 import dub.internal.logging;
 import dub.package_;
+import dub.recipe.io;
+import configy.Exceptions;
+public import configy.Read : StrictMode;
 
 import std.algorithm : countUntil, filter, sort, canFind, remove;
 import std.array;
@@ -254,17 +257,19 @@ class PackageManager {
 			path = NativePath to the root directory of the package
 			recipe_path = Optional path to the recipe file of the package
 			allow_sub_packages = Also return a sub package if it resides in the given folder
+			mode = Whether to issue errors, warning, or ignore unknown keys in dub.json
 
 		Returns: The packages loaded from the given path
 		Throws: Throws an exception if no package can be loaded
 	*/
-	Package getOrLoadPackage(NativePath path, NativePath recipe_path = NativePath.init, bool allow_sub_packages = false)
+	Package getOrLoadPackage(NativePath path, NativePath recipe_path = NativePath.init,
+		bool allow_sub_packages = false, StrictMode mode = StrictMode.Ignore)
 	{
 		path.endsWithSlash = true;
 		foreach (p; getPackageIterator())
 			if (p.path == path && (!p.parentPackage || (allow_sub_packages && p.parentPackage.path != p.path)))
 				return p;
-		auto pack = Package.load(path, recipe_path);
+		auto pack = Package.load(path, recipe_path, null, null, mode);
 		addPackages(m_temporaryPackages, pack);
 		return pack;
 	}
@@ -790,6 +795,9 @@ class PackageManager {
 								}
 						if (!p) p = Package.load(pack_path, packageFile);
 						addPackages(m_packages, p);
+					} catch (ConfigException exc) {
+						// Confiy error message already include the path
+						logError("Invalid recipe for local package: %S", exc);
 					} catch( Exception e ){
 						logError("Failed to load package in %s: %s", pack_path, e.msg);
 						logDiagnostic("Full error: %s", e.toString().sanitize());
diff --git a/source/dub/project.d b/source/dub/project.d
@@ -65,7 +65,7 @@ class Project {
 			logWarn("There was no package description found for the application in '%s'.", project_path.toNativeString());
 			pack = new Package(PackageRecipe.init, project_path);
 		} else {
-			pack = package_manager.getOrLoadPackage(project_path, packageFile);
+			pack = package_manager.getOrLoadPackage(project_path, packageFile, false, StrictMode.Warn);
 		}
 
 		this(package_manager, pack);
diff --git a/source/dub/recipe/io.d b/source/dub/recipe/io.d
@@ -8,8 +8,9 @@
 module dub.recipe.io;
 
 import dub.recipe.packagerecipe;
+import dub.internal.logging;
 import dub.internal.vibecompat.inet.path;
-
+import configy.Read;
 
 /** Reads a package recipe from a file.
 
@@ -18,16 +19,20 @@ import dub.internal.vibecompat.inet.path;
 	Params:
 		filename = NativePath of the package recipe file
 		parent_name = Optional name of the parent package (if this is a sub package)
+		mode = Whether to issue errors, warning, or ignore unknown keys in dub.json
 
 	Returns: Returns the package recipe contents
 	Throws: Throws an exception if an I/O or syntax error occurs
 */
-PackageRecipe readPackageRecipe(string filename, string parent_name = null)
+PackageRecipe readPackageRecipe(
+	string filename, string parent_name = null, StrictMode mode = StrictMode.Ignore)
 {
-	return readPackageRecipe(NativePath(filename), parent_name);
+	return readPackageRecipe(NativePath(filename), parent_name, mode);
 }
+
 /// ditto
-PackageRecipe readPackageRecipe(NativePath filename, string parent_name = null)
+PackageRecipe readPackageRecipe(
+	NativePath filename, string parent_name = null, StrictMode mode = StrictMode.Ignore)
 {
 	import dub.internal.utils : stripUTF8Bom;
 	import dub.internal.vibecompat.core.file : openFile, FileMode;
@@ -40,7 +45,7 @@ PackageRecipe readPackageRecipe(NativePath filename, string parent_name = null)
 		text = stripUTF8Bom(cast(string)f.readAll());
 	}
 
-	return parsePackageRecipe(text, filename.toNativeString(), parent_name);
+	return parsePackageRecipe(text, filename.toNativeString(), parent_name, null, mode);
 }
 
 /** Parses an in-memory package recipe.
@@ -55,12 +60,13 @@ PackageRecipe readPackageRecipe(NativePath filename, string parent_name = null)
 		package)
 		default_package_name = Optional default package name (if no package name
 		is found in the recipe this value will be used)
+		mode = Whether to issue errors, warning, or ignore unknown keys in dub.json
 
 	Returns: Returns the package recipe contents
 	Throws: Throws an exception if an I/O or syntax error occurs
 */
 PackageRecipe parsePackageRecipe(string contents, string filename, string parent_name = null,
-								 string default_package_name = null)
+	string default_package_name = null, StrictMode mode = StrictMode.Ignore)
 {
 	import std.algorithm : endsWith;
 	import dub.compilers.buildsettings : TargetType;
@@ -72,7 +78,46 @@ PackageRecipe parsePackageRecipe(string contents, string filename, string parent
 
 	ret.name = default_package_name;
 
-	if (filename.endsWith(".json")) parseJson(ret, parseJsonString(contents, filename), parent_name);
+	if (filename.endsWith(".json"))
+	{
+		try {
+			ret = parseConfigString!PackageRecipe(contents, filename, mode);
+			fixDependenciesNames(ret.name, ret);
+		} catch (ConfigException exc) {
+			logWarn("Your `dub.json` file use non-conventional features that are deprecated");
+			logWarn("Please adjust your `dub.json` file as those warnings will turn into errors in dub v1.40.0");
+			logWarn("Error was: %s", exc);
+			// Fallback to JSON parser
+			ret = PackageRecipe.init;
+			parseJson(ret, parseJsonString(contents, filename), parent_name);
+		} catch (Exception exc) {
+			logWarn("Your `dub.json` file use non-conventional features that are deprecated");
+			logWarn("This is most likely due to duplicated keys.");
+			logWarn("Please adjust your `dub.json` file as those warnings will turn into errors in dub v1.40.0");
+			logWarn("Error was: %s", exc);
+			// Fallback to JSON parser
+			ret = PackageRecipe.init;
+			parseJson(ret, parseJsonString(contents, filename), parent_name);
+		}
+		// `debug = ConfigFillerDebug` also enables verbose parser output
+		debug (ConfigFillerDebug)
+		{
+			import std.stdio;
+
+			PackageRecipe jsonret;
+			parseJson(jsonret, parseJsonString(contents, filename), parent_name);
+			if (ret != jsonret)
+			{
+				writeln("Content of JSON and YAML parsing differ for file: ", filename);
+				writeln("-------------------------------------------------------------------");
+				writeln("JSON (excepted): ", jsonret);
+				writeln("-------------------------------------------------------------------");
+				writeln("YAML (actual  ): ", ret);
+				writeln("========================================");
+				ret = jsonret;
+			}
+		}
+	}
 	else if (filename.endsWith(".sdl")) parseSDL(ret, contents, parent_name, filename);
 	else assert(false, "readPackageRecipe called with filename with unknown extension: "~filename);
 
@@ -194,3 +239,56 @@ void serializePackageRecipe(R)(ref R dst, const scope ref PackageRecipe recipe,
 		toSDL(recipe).toSDLDocument(dst);
 	else assert(false, "writePackageRecipe called with filename with unknown extension: "~filename);
 }
+
+unittest {
+	import std.format;
+	import dub.dependency;
+	import dub.internal.utils : deepCompare;
+
+	static void success (string source, in PackageRecipe expected, size_t line = __LINE__) {
+		const result = parseConfigString!PackageRecipe(source, "dub.json");
+		deepCompare(result, expected, __FILE__, line);
+	}
+
+	static void error (string source, string expected, size_t line = __LINE__) {
+		try
+		{
+			auto result = parseConfigString!PackageRecipe(source, "dub.json");
+			assert(0,
+				   format("[%s:%d] Exception should have been thrown but wasn't: %s",
+						  __FILE__, line, result));
+		}
+		catch (Exception exc)
+			assert(exc.toString() == expected,
+				   format("[%s:%s] result != expected: '%s' != '%s'",
+						  __FILE__, line, exc.toString(), expected));
+	}
+
+	alias YAMLDep = typeof(BuildSettingsTemplate.dependencies[string.init]);
+	const PackageRecipe expected1 =
+	{
+		name: "foo",
+		buildSettings: {
+		dependencies: RecipeDependencyAA([
+			"repo": YAMLDep(Dependency(Repository(
+				"git+https://github.com/dlang/dmd",
+				"09d04945bdbc0cba36f7bb1e19d5bd009d4b0ff2",
+			))),
+			"path": YAMLDep(Dependency(NativePath("/foo/bar/jar/"))),
+			"version": YAMLDep(Dependency(VersionRange.fromString("~>1.0"))),
+			"version2": YAMLDep(Dependency(Version("4.2.0"))),
+		])},
+	};
+	success(
+		`{ "name": "foo", "dependencies": {
+	"repo": { "repository": "git+https://github.com/dlang/dmd",
+			  "version": "09d04945bdbc0cba36f7bb1e19d5bd009d4b0ff2" },
+	"path":    { "path": "/foo/bar/jar/" },
+	"version": { "version": "~>1.0" },
+	"version2": "4.2.0"
+}}`, expected1);
+
+
+	error(`{ "name": "bar", "dependencies": {"bad": { "repository": "git+https://github.com/dlang/dmd" }}}`,
+		"dub.json(0:41): dependencies[bad]: Need to provide a commit hash in 'version' field with 'repository' dependency");
+}
diff --git a/source/dub/recipe/packagerecipe.d b/source/dub/recipe/packagerecipe.d

Original file line number	Diff line number	Diff line change
`@@ -65,7 +65,7 @@ class Project {`
`65`	`65`	`logWarn("There was no package description found for the application in '%s'.", project_path.toNativeString());`
`66`	`66`	`pack = new Package(PackageRecipe.init, project_path);`
`67`	`67`	`} else {`
`68`		`- pack = package_manager.getOrLoadPackage(project_path, packageFile);`
	`68`	`+ pack = package_manager.getOrLoadPackage(project_path, packageFile, false, StrictMode.Warn);`
`69`	`69`	`}`
`70`	`70`
`71`	`71`	`this(package_manager, pack);`