You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by rm...@apache.org on 2021/08/16 10:47:01 UTC

[maven-shade-plugin] 01/02: [MSHADE-396] Improve SourceContent Shading

This is an automated email from the ASF dual-hosted git repository.

rmannibucau pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-shade-plugin.git

commit 1dca37c71649fa4f4aa110de3b78cc98d0e47eda
Author: Alexander Kriegisch <Al...@Kriegisch.name>
AuthorDate: Mon Jul 12 14:44:53 2021 +0700

    [MSHADE-396] Improve SourceContent Shading
    
    Improve search & replace heuristics without destroying previously
    correct replacements in my test project (AspectJ). This solution is
    still bound to fail in some situations, simply because it is just a
    heuristic approach and not a full Java parser correctly recognising
    package names in all possible situations in Java source code. As for
    matching within Java string constants, this is next to impossible to get
    100% right.
    
    But 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.
---
 .../plugins/shade/relocation/SimpleRelocator.java  | 41 ++++++++++++++-----
 .../shade/relocation/SimpleRelocatorTest.java      | 46 ++++++++++++++++++----
 2 files changed, 70 insertions(+), 17 deletions(-)

diff --git a/src/main/java/org/apache/maven/plugins/shade/relocation/SimpleRelocator.java b/src/main/java/org/apache/maven/plugins/shade/relocation/SimpleRelocator.java
index 083957f..3837a67 100644
--- a/src/main/java/org/apache/maven/plugins/shade/relocation/SimpleRelocator.java
+++ b/src/main/java/org/apache/maven/plugins/shade/relocation/SimpleRelocator.java
@@ -34,6 +34,23 @@ import java.util.regex.Pattern;
 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 class SimpleRelocator
         {
             this.includes.addAll( includes );
         }
-        
+
         if ( excludes != null && !excludes.isEmpty() )
         {
             this.excludes.addAll( excludes );
@@ -121,7 +138,7 @@ public class SimpleRelocator
                     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 class SimpleRelocator
         {
             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 @@ public class SimpleRelocator
         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 @@ public class SimpleRelocator
             {
                 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();
diff --git a/src/test/java/org/apache/maven/plugins/shade/relocation/SimpleRelocatorTest.java b/src/test/java/org/apache/maven/plugins/shade/relocation/SimpleRelocatorTest.java
index 2934d0d..e85972a 100644
--- a/src/test/java/org/apache/maven/plugins/shade/relocation/SimpleRelocatorTest.java
+++ b/src/test/java/org/apache/maven/plugins/shade/relocation/SimpleRelocatorTest.java
@@ -8,9 +8,9 @@ package org.apache.maven.plugins.shade.relocation;
  * 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 class SimpleRelocatorTest
 
         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 class SimpleRelocatorTest
         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 class SimpleRelocatorTest
 
     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 class SimpleRelocatorTest
             "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 class SimpleRelocatorTest
 
     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 class SimpleRelocatorTest
             "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 class SimpleRelocatorTest
     @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 )
+                    )
+                )
+            )
+        );
     }
 }