Plugin target-dir error handling for source-file element

[brody4hire]

In #576 & #577 there is an open question about what if a character other than / comes after a target-dir prefix such as app, libs, or src/main in a source-file element.

I think we should consider aborting with an error in this case and in case of some other invalid target-dir prefixes.

I wonder if we should consider a similar form of error handling on the other platforms.

[Jule-]

As mentioned in #576 I made a draft for this function with some unknown variables from my point of vue. For clarity and readability of this issue I copy it here:

function getInstallDestination(obj) {
  var APP_MAIN_PREFIX = 'app/src/main';

  if (obj.targetDir.startsWith('app/src/main/')) {
    // plugin targets explicitly source file root => use targetDir as it is.
    return path.join(obj.targetDir, path.basename(obj.src));
  } else {
    // plugin ignores underlying android app structure
    // @deprecated: BACKWARD COMPATIBILITY

    // I don't have enough history on this part so I don't know if there is specific
    // folder to put files into in order to compile correctly but I guess:
    if (obj.src.endsWith('.java')) {
      return path.join(APP_MAIN_PREFIX, 'java', obj.targetDir.replace(/^src\//, ''),
        path.basename(obj.src));
    } else if (obj.src.endsWith('.aidl')) {
      return path.join(APP_MAIN_PREFIX, 'aidl', obj.targetDir.replace(/^src\//, ''),
        path.basename(obj.src));
    } else if (obj.targetDir.startsWith('libs/')) {
      if (obj.src.endsWith('.so')) {
        // Here we should have kept the old form with .substring(5) since it is safe 
        // after .startsWith('libs/').
        // But I don't think we need optimization here more than readability.
        return path.join(APP_MAIN_PREFIX, 'jniLibs', obj.targetDir.replace(/^libs\//, ''),
          path.basename(obj.src));
      } else {
        // Don't know what sort of lib this should be handling
        return path.join('app', obj.targetDir, path.basename(obj.src));
      }
    } else if (obj.targetDir.startsWith('src/main/')) {
      // Seems to be an awkward form handling
      return path.join('app', obj.targetDir, path.basename(obj.src));
    }

    // For all other source files not using the new app directory structure,
    // add 'app/src/main' to the targetDir
    return path.join(APP_MAIN_PREFIX, obj.targetDir, path.basename(obj.src));
  }
}

[Jule-]

@brodybits Indeed I think we should put warnings for deprecated forms in order to remove them later and in order than the community could trigger PR on plugin maintainers before breaking all that stuff.

[brody4hire]

The forms in the description of this issue should be considered corner cases, which rarely happen in the real world. In these cases I can think of the following options:

• emit a warning and continue with some form of behavior
• fail with an error message
• ignore them

If we would just emit a warning what kind of behavior do you think we should have?

If you want to propose more changes to getInstallDestination I suggest you open a new PR where we can discuss in more detail, line-by-line where needed.