Remove incorrect package structure warnings. (#1236)

* package structure tests that were incorrect now are correct

* remove validatePackageStructure proc

* fix failing test

* remove one test because it produces a warning

Warning: Symlink already exists in /home/runner/work/nimble/nimble/tests/nimbleDir/bin/y. Replacing.

* remove from list

src/nimblepkg/packageparser.nim

@@ -90,65 +90,6 @@ proc validateVersion*(ver: string) =
           "Version may only consist of numbers and the '.' character " &
           "but found '" & c & "'.", false)
 
-proc validatePackageStructure(pkgInfo: PackageInfo, options: Options) =
-  ## This ensures that a package's source code does not leak into
-  ## another package's namespace.
-  ## https://github.com/nim-lang/nimble/issues/144
-  let
-    realDir = pkgInfo.getRealDir()
-    correctDir = pkgInfo.basicInfo.name
-
-  proc onFile(path: string) =
-    # Remove the root to leave only the package subdirectories.
-    # ~/package-0.1/package/utils.nim -> package/utils.nim.
-    var trailPath = changeRoot(realDir, "", path)
-    if trailPath.startsWith(DirSep): trailPath = trailPath[1 .. ^1]
-    let (dir, file, ext) = trailPath.splitFile
-    # We're only interested in nim files, because only they can pollute our
-    # namespace.
-    if ext != (ExtSep & "nim"):
-      return
-
-    if dir.len == 0:
-      if file != pkgInfo.basicInfo.name:
-        # A source file was found in the top level of srcDir that doesn't share
-        # a name with the package.
-        let
-          msg = ("Package '$1' has an incorrect structure. " &
-                "The top level of the package source directory " &
-                "should contain at most one module, " &
-                "named '$2', but a file named '$3' was found. This " &
-                "will be an error in the future.") %
-                [pkgInfo.basicInfo.name, pkgInfo.basicInfo.name & ext, file & ext]
-          hint = ("If this is the primary source file in the package, " &
-                  "rename it to '$1'. If it's a source file required by " &
-                  "the main module, or if it is one of several " &
-                  "modules exposed by '$4', then move it into a '$2' subdirectory. " &
-                  "If it's a test file or otherwise not required " &
-                  "to build the package '$1', prevent its installation " &
-                  "by adding `skipFiles = @[\"$3\"]` to the .nimble file. See " &
-                  "https://github.com/nim-lang/nimble#libraries for more info.") %
-                  [pkgInfo.basicInfo.name & ext, correctDir & DirSep, file & ext, pkgInfo.basicInfo.name]
-        raise validationError(msg, true, hint, true)
-    else:
-      assert(not pkgInfo.isMinimal)
-      # On Windows `pkgInfo.bin` has a .exe extension, so we need to normalize.
-      if not (dir.startsWith(correctDir & DirSep) or dir == correctDir):
-        let
-          msg = ("Package '$2' has an incorrect structure. " &
-                "It should contain a single directory hierarchy " &
-                "for source files, named '$3', but file '$1' " &
-                "is in a directory named '$4' instead. " &
-                "This will be an error in the future.") %
-              [file & ext, pkgInfo.basicInfo.name, correctDir, dir]
-          hint = ("If '$1' contains source files for building '$2', rename it " &
-                  "to '$3'. Otherwise, prevent its installation " &
-                  "by adding `skipDirs = @[\"$1\"]` to the .nimble file.") %
-              [dir, pkgInfo.basicInfo.name, correctDir]
-        raise validationError(msg, true, hint, true)
-
-  iterInstallFiles(realDir, pkgInfo, options, onFile)
-
 proc validatePackageInfo(pkgInfo: PackageInfo, options: Options) =
   let path = pkgInfo.myPath
   if pkgInfo.basicInfo.name == "":
@@ -176,10 +117,6 @@ proc validatePackageInfo(pkgInfo: PackageInfo, options: Options) =
     if pkgInfo.backend notin ["c", "cc", "objc", "cpp", "js"]:
       raise validationError("'" & pkgInfo.backend &
           "' is an invalid backend.", false)
-  if options.action.typ in {actionInstall, actionBuild, actionDevelop, actionCompile, actionCheck}:
-    # nim is used for building the project, thus no need to validate its structure.
-    if not pkgInfo.basicInfo.name.isNim:
-      validatePackageStructure(pkginfo, options)
 
 proc nimScriptHint*(pkgInfo: PackageInfo) =
   if not pkgInfo.isNimScript:

tests/packageStructure/y/y.nimble

@@ -2,7 +2,7 @@
 
 version       = "0.1.0"
 author        = "Dominik Picheta"
-description   = "Incorrectly structured package Y"
+description   = "Correctly structured package Y"
 license       = "MIT"
 
 installExt = @["nim"]

tests/packageStructure/z/z.nimble

@@ -2,7 +2,7 @@
 
 version       = "0.1.0"
 author        = "Dominik Picheta"
-description   = "Incorrect package structure Z."
+description   = "Correct package structure Z."
 license       = "MIT"
 
 # Dependencies

tests/tcheckcommand.nim

@@ -33,10 +33,8 @@ suite "check command":
       check outp.processOutput.inLines("success")
       check outp.processOutput.inLines("\"c\" is valid")
 
-  test "can fail package":
     cd "packageStructure/x":
       let (outp, exitCode) = execNimble("check")
-      check exitCode == QuitFailure
-      check outp.processOutput.inLines("failure")
-      check outp.processOutput.inLines("validation failed")
-      check outp.processOutput.inLines("package 'x' has an incorrect structure")
+      check exitCode == QuitSuccess
+      check outp.processOutput.inLines("success")
+      check outp.processOutput.inLines("\"x\" is valid")

tests/tissues.nim

@@ -306,41 +306,13 @@ suite "issues":
 
   test "can validate package structure (#144)":
     # Test that no warnings are produced for correctly structured packages.
-    for package in ["a", "b", "c", "validBinary", "softened"]:
+    for package in ["a", "b", "c", "softened", "x", "y", "z"]:
       cd "packageStructure/" & package:
         let (output, exitCode) = execNimbleYes("install")
         check exitCode == QuitSuccess
         let lines = output.strip.processOutput()
         check(not lines.hasLineStartingWith("Warning:"))
 
-    # Test that warnings are produced for the incorrectly structured packages.
-    for package in ["x", "y", "z"]:
-      cd "packageStructure/" & package:
-        let (output, exitCode) = execNimbleYes("install")
-        check exitCode == QuitSuccess
-        let lines = output.strip.processOutput()
-        checkpoint(output)
-        case package
-        of "x":
-          check lines.hasLineStartingWith(
-            "Warning: Package 'x' has an incorrect structure. It should" &
-            " contain a single directory hierarchy for source files," &
-            " named 'x', but file 'foobar.nim' is in a directory named" &
-            " 'incorrect' instead.")
-        of "y":
-          check lines.hasLineStartingWith(
-            "Warning: Package 'y' has an incorrect structure. It should" &
-            " contain a single directory hierarchy for source files," &
-            " named 'y', but file 'foobar.nim' is in a directory named" &
-            " 'yWrong' instead.")
-        of "z":
-          check lines.hasLineStartingWith(
-            "Warning: Package 'z' has an incorrect structure. The top level" &
-            " of the package source directory should contain at most one module," &
-            " named 'z.nim', but a file named 'incorrect.nim' was found.")
-        else:
-          assert false
-
   test "issue 129 (installing commit hash)":
     cleanDir(installDir)
     let arguments = @["install", &"{pkgAUrl}@#1f9cb289c89"]