Skip to content

bug(#4529): locals-to-aliases optimization implemented in MjParse #4588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
h1alexbel wants to merge 2 commits into
base: master
Choose a base branch
from
+98 −48
Draft

bug(#4529): locals-to-aliases optimization implemented in MjParse #4588

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
af4ee41
bug(#4529): add XSL transformation to convert local objects to aliase…
h1alexbel Sep 25, 2025
0c6fea8
bug(#4529): replace XSL transformation with Xsline for locals-to-alia…
h1alexbel Sep 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion eo-maven-plugin/src/main/java/org/eolang/maven/MjParse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
import com.jcabi.log.Logger;
import com.jcabi.xml.XML;
import com.jcabi.xml.XMLDocument;
import com.yegor256.xsline.StClasspath;
import com.yegor256.xsline.TrDefault;
import com.yegor256.xsline.Xsline;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.nio.file.Path;
Expand All @@ -21,6 +24,7 @@
import org.apache.maven.plugins.annotations.ResolutionScope;
import org.cactoos.io.InputOf;
import org.cactoos.iterable.Filtered;
import org.cactoos.list.ListOf;
import org.cactoos.text.TextOf;
import org.eolang.parser.EoSyntax;
import org.eolang.parser.OnDefault;
Expand All @@ -41,6 +45,32 @@
requiresDependencyResolution = ResolutionScope.COMPILE
)
public final class MjParse extends MjSafe {
/**
* Reserved object bases from QQ.
*/
private static final String QQS = String.join(
",",
new ListOf<>(
"Φ.org.eolang.number",
"Φ.org.eolang.bytes"
)
);

Comment on lines +51 to +58
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix Checkstyle DeclarationOrder by aligning access modifiers.

The new fields precede package-private constants (ZERO, DIR, CACHE), causing “Variable access definition in wrong order.” Make these new constants package‑private to match, or move them below. Quick unblocking change:

Apply this diff:

-    private static final String QQS = String.join(
+    static final String QQS = String.join(
         ",",
         new ListOf<>(
             "Φ.org.eolang.number",
             "Φ.org.eolang.bytes"
         )
     );
🤖 Prompt for AI Agents 
In eo-maven-plugin/src/main/java/org/eolang/maven/MjParse.java around lines 51
to 58, the newly added constants are declared private which violates Checkstyle
DeclarationOrder because they appear before package-private constants (ZERO,
DIR, CACHE); change these new constants to package-private by removing the
'private' modifier (or alternatively move their declarations below the existing
package-private constants) so their access modifiers align with the existing
fields and resolve the "Variable access definition in wrong order" error.

/**
* Optimization line.
*/
private static final Xsline OPTIMIZATION_LINE = new Xsline(
new TrDefault<>(
new StClasspath(
"/org/eolang/maven/parse/locals-to-aliases.xsl",
"name reserved",
String.format(
"reserved %s",
MjParse.QQS
)
)
)
);
Comment on lines +62 to +73
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Same Checkstyle issue for OPTIMIZATION_LINE; also drop unused XSL param “name”.

  • Align access to package‑private to satisfy DeclarationOrder.
  • The XSL doesn’t use the “name” param; remove it to avoid confusion.

Apply this diff:

-    private static final Xsline OPTIMIZATION_LINE = new Xsline(
+    static final Xsline OPTIMIZATION_LINE = new Xsline(
         new TrDefault<>(
             new StClasspath(
                 "/org/eolang/maven/parse/locals-to-aliases.xsl",
-                "name reserved",
                 String.format(
                     "reserved %s",
                     MjParse.QQS
                 )
             )
         )
     );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private static final Xsline OPTIMIZATION_LINE = new Xsline(
new TrDefault<>(
new StClasspath(
"/org/eolang/maven/parse/locals-to-aliases.xsl",
"name reserved",
String.format(
"reserved %s",
MjParse.QQS
)
)
)
);
static final Xsline OPTIMIZATION_LINE = new Xsline(
new TrDefault<>(
new StClasspath(
"/org/eolang/maven/parse/locals-to-aliases.xsl",
String.format(
"reserved %s",
MjParse.QQS
)
)
)
);
🤖 Prompt for AI Agents 
In eo-maven-plugin/src/main/java/org/eolang/maven/MjParse.java around lines 62
to 73, make OPTIMIZATION_LINE package-private (remove the "private" modifier) to
align with DeclarationOrder and remove the unused XSL parameter "name" (drop the
second argument "name reserved" from the StClasspath constructor call), leaving
only the resource path and the actual "reserved ..." parameter; keep the rest of
the Xsline/TrDefault wrapping unchanged.


/**
* Zero version.
Expand Down Expand Up @@ -104,7 +134,7 @@ private int parsed(final TjForeign tojo) throws Exception {
src -> {
final Node node = this.parsed(src, name);
refs.add(node);
return new XMLDocument(node).toString();
return MjParse.OPTIMIZATION_LINE.pass(new XMLDocument(node)).toString();
},
this.cache.toPath().resolve(MjParse.CACHE),
this.plugin.getVersion(),
Expand Down
39 changes: 39 additions & 0 deletions eo-maven-plugin/src/main/resources/org/eolang/maven/parse/locals-to-aliases.xsl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
* SPDX-FileCopyrightText: Copyright (c) 2016-2025 Objectionary.com
* SPDX-License-Identifier: MIT
-->
<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform" xmlns:eo="https://www.eolang.org" id="locals-to-aliases" version="2.0">
<xsl:param name="name"/>
<xsl:param name="reserved"/>
<xsl:output encoding="UTF-8" method="xml"/>
<xsl:variable name="pkg" select="normalize-space(/object/metas/meta[head='package'][1]/tail)"/>
<xsl:variable name="qqs" select="tokenize(normalize-space($reserved), '[\s,]+')"/>
<xsl:template match="@*|node()">
<xsl:copy>
<xsl:apply-templates select="@*|node()"/>
</xsl:copy>
</xsl:template>
<xsl:template match="@base[starts-with(., 'Φ.') and not(. = $qqs)]">
<xsl:variable name="seg" select="tokenize(., '\.')[last()]"/>
<xsl:attribute name="base" select="concat($pkg, '.', $seg)"/>
</xsl:template>
Comment on lines +17 to +20
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Wrong rewrite target (affects Φ. instead of locals).*

You currently rewrite bases starting with Φ., which are standard library objects, not locals. This will incorrectly turn many QQ references into package ones and won’t touch local names like “bar”. Rewrite only simple, local bases (no dot, not Φ/ξ).

Apply this diff:

-  <xsl:template match="@base[starts-with(., 'Φ.') and not(. = $qqs)]">
-    <xsl:variable name="seg" select="tokenize(., '\.')[last()]"/>
-    <xsl:attribute name="base" select="concat($pkg, '.', $seg)"/>
-  </xsl:template>
+  <!-- Rewrite only local (simple) bases to fully-qualified ones -->
+  <xsl:template match="@base[not(contains(., '.')) and not(starts-with(., 'Φ')) and not(starts-with(., 'ξ')) and string-length($pkg) &gt; 0]">
+    <xsl:attribute name="base" select="concat($pkg, '.', .)"/>
+  </xsl:template>
🤖 Prompt for AI Agents 
In
eo-maven-plugin/src/main/resources/org/eolang/maven/parse/locals-to-aliases.xsl
around lines 17-20, the template currently matches bases starting with "Φ."
(affecting standard library names) and tokenizes by dots; instead, change the
match to target only simple local bases (no dot, and not the special names Φ or
ξ) and exclude $qqs. Replace the condition with something like
@base[not(contains(., '.')) and not(. = 'Φ') and not(. = 'ξ') and not(. = $qqs)]
and set the new base to concat($pkg, '.', .) (remove the tokenize/last() logic).
Ensure the attribute creation uses the simple local name rather than extracting
the last dotted segment.

<xsl:template match="metas">
<xsl:copy>
<xsl:apply-templates select="@*|node()"/>
<xsl:for-each-group select="//@base[starts-with(., 'Φ.') and not(. = $qqs)]" group-by="tokenize(., '\.')[last()]">
<xsl:variable name="seg" select="current-grouping-key()"/>
<xsl:variable name="new" select="concat($pkg, '.', $seg)"/>
<xsl:variable name="tail" select="concat($seg, ' ', $new)"/>
<xsl:if test="not(/object/metas/meta[head='alias' and tail=$new])">
<meta>
<head>alias</head>
<tail><xsl:value-of select="$tail"/></tail>
<part><xsl:value-of select="$seg"/></part>
<part><xsl:value-of select="$new"/></part>
</meta>
</xsl:if>
</xsl:for-each-group>
</xsl:copy>
Comment on lines +21 to +37
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Alias generation targets the wrong set and has a bug in existence check.

  • It groups Φ.* bases, so it will never create an alias for local “bar”.
  • It checks existing aliases by tail=$new instead of the full “bar foo.bar”.

Apply this diff:

   <xsl:template match="metas">
     <xsl:copy>
       <xsl:apply-templates select="@*|node()"/>
-      <xsl:for-each-group select="//@base[starts-with(., 'Φ.') and not(. = $qqs)]" group-by="tokenize(., '\.')[last()]">
-        <xsl:variable name="seg"  select="current-grouping-key()"/>
-        <xsl:variable name="new"  select="concat($pkg, '.', $seg)"/>
-        <xsl:variable name="tail" select="concat($seg, ' ', $new)"/>
-        <xsl:if test="not(/object/metas/meta[head='alias' and tail=$new])">
+      <!-- Create aliases for local names only -->
+      <xsl:for-each-group
+          select="//@base[not(contains(., '.')) and not(starts-with(., 'Φ')) and not(starts-with(., 'ξ'))]"
+          group-by=".">
+        <xsl:variable name="seg"  select="current-grouping-key()"/>
+        <xsl:variable name="new"  select="concat($pkg, '.', $seg)"/>
+        <xsl:variable name="tail" select="concat($seg, ' ', $new)"/>
+        <xsl:if test="string-length($pkg) &gt; 0 and not(/object/metas/meta[head='alias' and tail=$tail])">
           <meta>
             <head>alias</head>
-            <tail><xsl:value-of select="$tail"/></tail>
-            <part><xsl:value-of select="$seg"/></part>
-            <part><xsl:value-of select="$new"/></part>
+            <tail><xsl:value-of select="$tail"/></tail>
+            <part><xsl:value-of select="$seg"/></part>
+            <part><xsl:value-of select="$new"/></part>
           </meta>
         </xsl:if>
       </xsl:for-each-group>
     </xsl:copy>
   </xsl:template>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<xsl:template match="metas">
<xsl:copy>
<xsl:apply-templates select="@*|node()"/>
<xsl:for-each-group select="//@base[starts-with(., 'Φ.') and not(. = $qqs)]" group-by="tokenize(., '\.')[last()]">
<xsl:variable name="seg" select="current-grouping-key()"/>
<xsl:variable name="new" select="concat($pkg, '.', $seg)"/>
<xsl:variable name="tail" select="concat($seg, ' ', $new)"/>
<xsl:if test="not(/object/metas/meta[head='alias' and tail=$new])">
<meta>
<head>alias</head>
<tail><xsl:value-of select="$tail"/></tail>
<part><xsl:value-of select="$seg"/></part>
<part><xsl:value-of select="$new"/></part>
</meta>
</xsl:if>
</xsl:for-each-group>
</xsl:copy>
<xsl:template match="metas">
<xsl:copy>
<xsl:apply-templates select="@*|node()"/>
<!-- Create aliases for local names only -->
<xsl:for-each-group
select="//@base[not(contains(., '.'))
and not(starts-with(., 'Φ'))
and not(starts-with(., 'ξ'))]"
group-by=".">
<xsl:variable name="seg" select="current-grouping-key()"/>
<xsl:variable name="new" select="concat($pkg, '.', $seg)"/>
<xsl:variable name="tail" select="concat($seg, ' ', $new)"/>
<xsl:if test="string-length($pkg) &gt; 0
and not(/object/metas/meta[head='alias' and tail=$tail])">
<meta>
<head>alias</head>
<tail><xsl:value-of select="$tail"/></tail>
<part><xsl:value-of select="$seg"/></part>
<part><xsl:value-of select="$new"/></part>
</meta>
</xsl:if>
</xsl:for-each-group>
</xsl:copy>
</xsl:template>
🤖 Prompt for AI Agents 
In
eo-maven-plugin/src/main/resources/org/eolang/maven/parse/locals-to-aliases.xsl
around lines 21 to 37, the for-each-group currently targets bases starting with
"Φ." (so it never creates aliases for plain local names like "bar") and the
existence check compares tail against $new instead of the full "seg new" string;
change the group selection to operate on bases that do NOT start with "Φ." (e.g.
select="//@base[not(starts-with(., 'Φ.')) and not(. = $qqs)]") so locals are
processed, and update the existence test to compare tail against the computed
$tail (not $new) so the meta-alias presence check is correct.

</xsl:template>
</xsl:stylesheet>
20 changes: 0 additions & 20 deletions eo-maven-plugin/src/main/resources/org/eolang/maven/shake/clean-up.xsl
View file
Open in desktop

This file was deleted.

27 changes: 0 additions & 27 deletions eo-maven-plugin/src/main/resources/org/eolang/maven/shake/remove-noise.xsl
View file
Open in desktop

This file was deleted.

28 changes: 28 additions & 0 deletions eo-maven-plugin/src/test/java/org/eolang/maven/MjParseTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package org.eolang.maven;

import com.jcabi.matchers.XhtmlMatchers;
import com.jcabi.xml.XML;
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Remove unused import to fix qulice failure.

com.jcabi.xml.XML is not used. This triggers the reported PMD/Checkstyle error.

Apply this diff:

-import com.jcabi.xml.XML;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import com.jcabi.xml.XML;
🧰 Tools
🪛 GitHub Actions: qulice

[error] 8-8: PMD: Unused import 'com.jcabi.xml.XML' (UnnecessaryImport)

🤖 Prompt for AI Agents 
In eo-maven-plugin/src/test/java/org/eolang/maven/MjParseTest.java around line
8, remove the unused import statement "com.jcabi.xml.XML" which is causing the
qulice/PMD/Checkstyle failure; delete that import line and re-run the
build/tests to confirm the linter error is resolved.

import com.jcabi.xml.XMLDocument;
import com.yegor256.Mktmp;
import com.yegor256.MktmpResolver;
Expand Down Expand Up @@ -283,6 +284,33 @@ void parsesWithTargetCache(@Mktmp final Path temp) throws IOException {
);
}

@Test
void convertsLocalObjectsToAliases(@Mktmp final Path temp) throws IOException {
MatcherAssert.assertThat(
"Local object is not converted to alias, but it should",
new XMLDocument(
new FakeMaven(temp)
.withProgram(
String.join(
"\n",
"+package foo",
"",
"[] > x",
" bar 42 > @"
),
"foo.x"
)
.execute(new FakeMaven.Parse())
.result()
.get("target/1-parse/foo/x.xmir")
),
XhtmlMatchers.hasXPaths(
"/object/metas/meta[head='alias' and part='bar' and part='foo.bar' and tail='bar foo.bar']",
"//o[@base='foo.bar']/o[1][@base='Φ.org.eolang.number']/o[1][@base='Φ.org.eolang.bytes']"
)
);
}

/**
* The mojo that does nothing, but executes infinitely.
* @since 0.29
Expand Down
Loading