[MSHADE-396] Improve SourceContent Shading

src/main/java/org/apache/maven/plugins/shade/mojo/ShadeMojo.java

@@ -327,9 +327,19 @@ public class ShadeMojo
     private boolean createTestSourcesJar;
 
     /**
-     * When true, it will attempt to shade the contents of the java source files when creating the sources jar. When
-     * false, it will just relocate the java source files to the shaded paths, but will not modify the actual contents
-     * of the java source files.
+     * When true, it will attempt to shade the contents of Java source files when creating the sources JAR. When false,
+     * it will just relocate the Java source files to the shaded paths, but will not modify the actual source file
+     * contents.
+     * <p>
+     * <b>Please note:</b> This feature uses a heuristic search & replace approach which covers many, but definitely not
+     * all possible cases of source code shading and its excludes. There is no full Java parser behind this
+     * functionality, which would be the only way to get this right for Java language elements. As for matching within
+     * Java string constants, this is next to impossible to get 100% right, trying to guess if they are used in
+     * reflection or not.
+     * <p>
+     * Please understand that the source shading feature is not meant as a source code generator anyway, merely as a
+     * tool creating reasonably plausible source code when navigating to a relocated library class from an IDE,
+     * hopefully displaying source code which makes 95% sense - no more, no less.
      */
     @Parameter( property = "shadeSourcesContent", defaultValue = "false" )
     private boolean shadeSourcesContent;

src/main/java/org/apache/maven/plugins/shade/relocation/SimpleRelocator.java

@@ -34,6 +34,23 @@
 public class SimpleRelocator
     implements Relocator
 {
+    /**
+     * Match dot, slash or space at end of string
+     */
+    private static final Pattern RX_ENDS_WITH_DOT_SLASH_SPACE = Pattern.compile( "[./ ]$" );
+
+    /**
+     * Match <ul>
+     *     <li>certain Java keywords + space</li>
+     *     <li>beginning of Javadoc link + optional line breaks and continuations with '*'</li>
+     * </ul>
+     * at end of string
+     */
+    private static final Pattern RX_ENDS_WITH_JAVA_KEYWORD = Pattern.compile(
+        "\\b(import|package|public|protected|private|static|final|synchronized|abstract|volatile) $"
+            + "|"
+            + "\\{@link( \\*)* $"
+    );
 
     private final String pattern;
 
@@ -104,7 +121,7 @@ public SimpleRelocator( String patt, String shadedPattern, List<String> includes
         {
             this.includes.addAll( includes );
         }
-        
+
         if ( excludes != null && !excludes.isEmpty() )
         {
             this.excludes.addAll( excludes );
@@ -121,7 +138,7 @@ public SimpleRelocator( String patt, String shadedPattern, List<String> includes
                     sourcePackageExcludes.add( exclude.substring( pattern.length() ).replaceFirst( "[.][*]$", "" ) );
                 }
                 // Excludes should be subpackages of the global pattern
-                else if ( exclude.startsWith( pathPattern ) )
+                if ( exclude.startsWith( pathPattern ) )
                 {
                     sourcePathExcludes.add( exclude.substring( pathPattern.length() ).replaceFirst( "[/][*]$", "" ) );
                 }
@@ -234,11 +251,8 @@ public String applyToSourceContent( String sourceContent )
         {
             return sourceContent;
         }
-        else
-        {
-            sourceContent = shadeSourceWithExcludes( sourceContent, pattern, shadedPattern, sourcePackageExcludes );
-            return shadeSourceWithExcludes( sourceContent, pathPattern, shadedPathPattern, sourcePathExcludes );
-        }
+        sourceContent = shadeSourceWithExcludes( sourceContent, pattern, shadedPattern, sourcePackageExcludes );
+        return shadeSourceWithExcludes( sourceContent, pathPattern, shadedPathPattern, sourcePathExcludes );
     }
 
     private String shadeSourceWithExcludes( String sourceContent, String patternFrom, String patternTo,
@@ -248,8 +262,11 @@ private String shadeSourceWithExcludes( String sourceContent, String patternFrom
         StringBuilder shadedSourceContent = new StringBuilder( sourceContent.length() * 11 / 10 );
         boolean isFirstSnippet = true;
         // Make sure that search pattern starts at word boundary and we look for literal ".", not regex jokers
-        for ( String snippet : sourceContent.split( "\\b" + patternFrom.replace( ".", "[.]" ) ) )
+        String[] snippets = sourceContent.split( "\\b" + patternFrom.replace( ".", "[.]" ) + "\\b" );
+        for ( int i = 0, snippetsLength = snippets.length; i < snippetsLength; i++ )
         {
+            String snippet = snippets[i];
+            String previousSnippet = isFirstSnippet ? "" : snippets[i - 1];
             boolean doExclude = false;
             for ( String excludedPattern : excludedPatterns )
             {
@@ -263,10 +280,14 @@ private String shadeSourceWithExcludes( String sourceContent, String patternFrom
             {
                 shadedSourceContent.append( snippet );
                 isFirstSnippet = false;
-        }
+            }
             else
             {
-                shadedSourceContent.append( doExclude ? patternFrom : patternTo ).append( snippet );
+                String previousSnippetOneLine = previousSnippet.replaceAll( "\\s+", " " );
+                boolean afterDotSlashSpace = RX_ENDS_WITH_DOT_SLASH_SPACE.matcher( previousSnippetOneLine ).find();
+                boolean afterJavaKeyWord = RX_ENDS_WITH_JAVA_KEYWORD.matcher( previousSnippetOneLine ).find();
+                boolean shouldExclude = doExclude || afterDotSlashSpace && !afterJavaKeyWord;
+                shadedSourceContent.append( shouldExclude ? patternFrom : patternTo ).append( snippet );
             }
         }
         return shadedSourceContent.toString();

src/test/java/org/apache/maven/plugins/shade/relocation/SimpleRelocatorTest.java

@@ -8,9 +8,9 @@
  * to you under the Apache License, Version 2.0 (the
  * "License"); you may not use this file except in compliance
  * with the License.  You may obtain a copy of the License at
- * 
+ *
  *   http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
  * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -103,7 +103,7 @@ public void testCanRelocateRawString()
 
         relocator = new SimpleRelocator( "org/foo", null, null, null, true );
         assertTrue( relocator.canRelocatePath( "(I)org/foo/bar/Class;" ) );
-        
+
         relocator = new SimpleRelocator( "^META-INF/org.foo.xml$", null, null, null, true );
         assertTrue( relocator.canRelocatePath( "META-INF/org.foo.xml" ) );
     }
@@ -173,7 +173,7 @@ public void testRelocateRawString()
         relocator = new SimpleRelocator( "^META-INF/org.foo.xml$", "META-INF/hidden.org.foo.xml", null, null, true );
         assertEquals( "META-INF/hidden.org.foo.xml", relocator.relocatePath( "META-INF/org.foo.xml" ) );
     }
-    
+
     @Test
     public void testRelocateMavenFiles()
     {
@@ -191,6 +191,7 @@ public void testRelocateMavenFiles()
 
     private static final String sourceFile =
             "package org.apache.maven.hello;\n" +
+            "package org.objectweb.asm;\n" +
             "\n" +
             "import foo.bar.Bar;\n" +
             "import zot.baz.Baz;\n" +
@@ -201,12 +202,19 @@ public void testRelocateMavenFiles()
             "import org.apache.maven.In;\n" +
             "import org.apache.maven.e.InE;\n" +
             "import org.apache.maven.f.g.InFG;\n" +
+            "import java.io.IOException;\n" +
             "\n" +
+            "/**\n" +
+            " * Also check out {@link org.apache.maven.hello.OtherClass} and {@link\n" +
+            " * org.apache.maven.hello.YetAnotherClass}\n" +
+            " */\n" +
             "public class MyClass {\n" +
             "  private org.apache.maven.exclude1.x.X myX;\n" +
-            "  private org.apache.maven.h.H;\n" +
+            "  private org.apache.maven.h.H h;\n" +
+            "  private String ioInput;\n" +
             "\n" +
             "  public void doSomething() {\n" +
+            "    String io, val;\n" +
             "    String noRelocation = \"NoWordBoundaryXXXorg.apache.maven.In\";\n" +
             "    String relocationPackage = \"org.apache.maven.In\";\n" +
             "    String relocationPath = \"org/apache/maven/In\";\n" +
@@ -215,6 +223,7 @@ public void testRelocateMavenFiles()
 
     private static final String relocatedFile =
             "package com.acme.maven.hello;\n" +
+            "package aj.org.objectweb.asm;\n" +
             "\n" +
             "import foo.bar.Bar;\n" +
             "import zot.baz.Baz;\n" +
@@ -225,12 +234,19 @@ public void testRelocateMavenFiles()
             "import com.acme.maven.In;\n" +
             "import com.acme.maven.e.InE;\n" +
             "import com.acme.maven.f.g.InFG;\n" +
+            "import java.io.IOException;\n" +
             "\n" +
+            "/**\n" +
+            " * Also check out {@link com.acme.maven.hello.OtherClass} and {@link\n" +
+            " * com.acme.maven.hello.YetAnotherClass}\n" +
+            " */\n" +
             "public class MyClass {\n" +
             "  private org.apache.maven.exclude1.x.X myX;\n" +
-            "  private com.acme.maven.h.H;\n" +
+            "  private com.acme.maven.h.H h;\n" +
+            "  private String ioInput;\n" +
             "\n" +
             "  public void doSomething() {\n" +
+            "    String io, val;\n" +
             "    String noRelocation = \"NoWordBoundaryXXXorg.apache.maven.In\";\n" +
             "    String relocationPackage = \"com.acme.maven.In\";\n" +
             "    String relocationPath = \"com/acme/maven/In\";\n" +
@@ -250,9 +266,25 @@ public void testRelocateSourceWithExcludesRaw()
     @Test
     public void testRelocateSourceWithExcludes()
     {
+        // Main relocator with in-/excludes
         SimpleRelocator relocator = new SimpleRelocator( "org.apache.maven", "com.acme.maven",
                 Arrays.asList( "foo.bar", "zot.baz" ),
                 Arrays.asList( "irrelevant.exclude", "org.apache.maven.exclude1", "org.apache.maven.sub.exclude2" ) );
-        assertEquals( relocatedFile,  relocator.applyToSourceContent( sourceFile ) );
+        // Make sure not to replace variables 'io' and 'ioInput', package 'java.io'
+        SimpleRelocator ioRelocator = new SimpleRelocator( "io", "shaded.io", null, null );
+        // Check corner case which was not working in PR #100
+        SimpleRelocator asmRelocator = new SimpleRelocator( "org.objectweb.asm", "aj.org.objectweb.asm", null, null );
+        // Make sure not to replace 'foo' package by path-like 'shaded/foo'
+        SimpleRelocator fooRelocator = new SimpleRelocator( "foo", "shaded.foo", null, Arrays.asList( "foo.bar" ) );
+        assertEquals(
+            relocatedFile,
+            fooRelocator.applyToSourceContent(
+                asmRelocator.applyToSourceContent(
+                    ioRelocator.applyToSourceContent(
+                        relocator.applyToSourceContent( sourceFile )
+                    )
+                )
+            )
+        );
     }
 }