From ebc0b5bdf6be6707deff062a62a2b8c750f53c83 Mon Sep 17 00:00:00 2001 From: Alexander Kriegisch Date: Mon, 12 Jul 2021 14:44:53 +0700 Subject: [PATCH 1/2] [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. --- .../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 083957fd..3837a67b 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 @@ 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 + * 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 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 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(); 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 2934d0d0..e85972a0 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 @@ * 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 ) + ) + ) + ) + ); } } From 80fa5aa99423d04bde6c4391631aed8d7ce1bf53 Mon Sep 17 00:00:00 2001 From: Alexander Kriegisch Date: Mon, 12 Jul 2021 16:01:33 +0700 Subject: [PATCH 2/2] [MSHADE-396] Add explanation to shadeSourcesContent documentation Explain purpose and limitations of heuristic source code shading approach. --- .../maven/plugins/shade/mojo/ShadeMojo.java | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/shade/mojo/ShadeMojo.java b/src/main/java/org/apache/maven/plugins/shade/mojo/ShadeMojo.java index 7c524f80..38125d2c 100644 --- a/src/main/java/org/apache/maven/plugins/shade/mojo/ShadeMojo.java +++ b/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. + *

+ * Please note: 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. + *

+ * 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;