-
Notifications
You must be signed in to change notification settings - Fork 17
Added linter for warning about unnecessary usage of ^ in code #754
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
67b0b68
3bb5795
b28765e
f85cda0
a392722
b1880ed
26ada99
0f2b0eb
55bef02
dc746e2
3775eb8
01a2191
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,93 @@ | ||||||||||||||||||
| /* | ||||||||||||||||||
| * SPDX-FileCopyrightText: Copyright (c) 2016-2025 Objectionary.com | ||||||||||||||||||
| * SPDX-License-Identifier: MIT | ||||||||||||||||||
| */ | ||||||||||||||||||
| package org.eolang.lints; | ||||||||||||||||||
|
|
||||||||||||||||||
| import com.github.lombrozo.xnav.Xnav; | ||||||||||||||||||
| import com.jcabi.xml.XML; | ||||||||||||||||||
| import java.io.IOException; | ||||||||||||||||||
| import java.util.ArrayList; | ||||||||||||||||||
| import java.util.Collection; | ||||||||||||||||||
| import java.util.List; | ||||||||||||||||||
| import java.util.stream.Collectors; | ||||||||||||||||||
| import org.cactoos.io.ResourceOf; | ||||||||||||||||||
| import org.cactoos.text.TextOf; | ||||||||||||||||||
| import org.cactoos.text.UncheckedText; | ||||||||||||||||||
| import org.eolang.parser.OnDefault; | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Lint that warns if a redundant {@code ^} is used. | ||||||||||||||||||
| * @since 0.0.59 | ||||||||||||||||||
| */ | ||||||||||||||||||
| final class LtRedundantRho implements Lint<XML> { | ||||||||||||||||||
| @Override | ||||||||||||||||||
| public Collection<Defect> defects(final XML xmir) throws IOException { | ||||||||||||||||||
Snehakb1219-christ marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||
| final Collection<Defect> defects = new ArrayList<>(0); | ||||||||||||||||||
| final Xnav xml = new Xnav(xmir.inner()); | ||||||||||||||||||
| final List<Xnav> objs = xml | ||||||||||||||||||
| .path("//o[starts-with(@base,'ξ.ρ')]") | ||||||||||||||||||
| .collect(Collectors.toList()); | ||||||||||||||||||
| for (final Xnav obj : objs) { | ||||||||||||||||||
|
Comment on lines
+28
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't match bare Bare Apply either of the following: Option A (selector): - .path("//o[starts-with(@base,'ξ.ρ')]")
+ .path("//o[starts-with(@base,'ξ.ρ.')]")Option B (guard): - for (final Xnav obj : objs) {
- final String name = obj.attribute("name").text().orElse("");
+ for (final Xnav obj : objs) {
+ final String base = obj.attribute("base").text().orElse("");
+ if ("ξ.ρ".equals(base) || !base.startsWith("ξ.ρ.")) {
+ continue; // ignore bare ^ or malformed cases
+ }📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
| final String name = obj.attribute("name").text().orElse(""); | ||||||||||||||||||
| final List<Xnav> matches = xml | ||||||||||||||||||
| .path(String.format("//o[@name='%s']", name)) | ||||||||||||||||||
| .collect(Collectors.toList()); | ||||||||||||||||||
|
Comment on lines
+32
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrong identifier extracted; use attribute after
Apply this diff to extract the correct attribute: - final String name = obj.attribute("name").text().orElse("");
+ final String base = obj.attribute("base").text().orElse("");
+ final String attr = base.startsWith("ξ.ρ.") ? base.substring("ξ.ρ.".length()) : "";
+ if (attr.isEmpty()) {
+ continue;
+ }Then replace subsequent usages of 🤖 Prompt for AI Agents |
||||||||||||||||||
| final List<Xnav> ancestors = obj | ||||||||||||||||||
| .path(String.format("ancestor::o[@name='%s']", name)) | ||||||||||||||||||
| .collect(Collectors.toList()); | ||||||||||||||||||
| if (matches.size() == 1) { | ||||||||||||||||||
| defects.add( | ||||||||||||||||||
| new Defect.Default( | ||||||||||||||||||
| this.name(), | ||||||||||||||||||
| Severity.WARNING, | ||||||||||||||||||
| new OnDefault(xmir).get(), | ||||||||||||||||||
| Integer.parseInt(obj.attribute("line").text().orElse("0")), | ||||||||||||||||||
| String.format( | ||||||||||||||||||
| "Redundant 'ξ.ρ' notation: '%s' can be accessed without it", | ||||||||||||||||||
| name | ||||||||||||||||||
| ) | ||||||||||||||||||
| ) | ||||||||||||||||||
| ); | ||||||||||||||||||
| } else if (!ancestors.isEmpty() && matches.size() > 1) { | ||||||||||||||||||
| final Xnav target = ancestors.get(0); | ||||||||||||||||||
| for (final Xnav match : matches) { | ||||||||||||||||||
| if (match.equals(target)) { | ||||||||||||||||||
| defects.add( | ||||||||||||||||||
| new Defect.Default( | ||||||||||||||||||
| this.name(), | ||||||||||||||||||
| Severity.WARNING, | ||||||||||||||||||
| new OnDefault(xmir).get(), | ||||||||||||||||||
| Integer.parseInt(obj.attribute("line").text().orElse("0")), | ||||||||||||||||||
| String.format( | ||||||||||||||||||
| "Redundant 'ξ.ρ' notation: '%s' resolves to the same object without it", | ||||||||||||||||||
| name | ||||||||||||||||||
| ) | ||||||||||||||||||
| ) | ||||||||||||||||||
| ); | ||||||||||||||||||
| break; | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+36
to
+71
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Redundancy check ignores local shadowing; produces false positives. Current logic counts all Use a scope-aware check: warn only if the immediate scope doesn’t define - final List<Xnav> matches = xml
- .path(String.format("//o[@name='%s']", name))
- .collect(Collectors.toList());
- final List<Xnav> ancestors = obj
- .path(String.format("ancestor::o[@name='%s']", name))
- .collect(Collectors.toList());
- if (matches.size() == 1) {
- defects.add(
- new Defect.Default(
- this.name(),
- Severity.WARNING,
- new OnDefault(xmir).get(),
- Integer.parseInt(obj.attribute("line").text().orElse("0")),
- String.format(
- "Redundant 'ξ.ρ' notation: '%s' can be accessed without it",
- name
- )
- )
- );
- } else if (!ancestors.isEmpty() && matches.size() > 1) {
- final Xnav target = ancestors.get(0);
- for (final Xnav match : matches) {
- if (match.equals(target)) {
- defects.add(
- new Defect.Default(
- this.name(),
- Severity.WARNING,
- new OnDefault(xmir).get(),
- Integer.parseInt(obj.attribute("line").text().orElse("0")),
- String.format(
- "Redundant 'ξ.ρ' notation: '%s' resolves to the same object without it",
- name
- )
- )
- );
- break;
- }
- }
- }
+ // Is there a local definition in the current scope that would shadow the parent?
+ final boolean hasLocal =
+ !obj.path(String.format("ancestor::o[1]/o[@name='%s']", attr))
+ .collect(Collectors.toList()).isEmpty();
+ // Does the parent scope define the attribute?
+ final boolean parentHasAttr =
+ !obj.path(String.format("ancestor::o[2]/o[@name='%s']", attr))
+ .collect(Collectors.toList()).isEmpty();
+ if (!hasLocal && parentHasAttr) {
+ defects.add(
+ new Defect.Default(
+ this.name(),
+ Severity.WARNING,
+ new OnDefault(xmir).get(),
+ Integer.parseInt(obj.attribute("line").text().orElse("0")),
+ String.format(
+ "Redundant 'ξ.ρ' notation: '%s' can be accessed without it",
+ attr
+ )
+ )
+ );
+ }Note: If XMIR nesting differs in your project, adjust
🤖 Prompt for AI Agents |
||||||||||||||||||
| } | ||||||||||||||||||
| return defects; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| @Override | ||||||||||||||||||
| public String motive() throws IOException { | ||||||||||||||||||
| return new UncheckedText( | ||||||||||||||||||
| new TextOf( | ||||||||||||||||||
| new ResourceOf( | ||||||||||||||||||
| String.format( | ||||||||||||||||||
| "org/eolang/motives/errors/%s.md", this.name() | ||||||||||||||||||
| ) | ||||||||||||||||||
| ) | ||||||||||||||||||
| ) | ||||||||||||||||||
| ).asString(); | ||||||||||||||||||
|
Comment on lines
+78
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainResource lookup: verify motive path and existence. Path hardcodes 🏁 Script executed: #!/bin/bash
# Verify motive file exists and is included.
fd -a 'redundant-hat.md' resources | sed -n '1p'
rg -n 'redundant-hat' -g resources/org/eolang/motives/**
# Also check the lint is registered in any aggregator/factory.
rg -nP '(redundant-hat|LtRedundantRho)' -g 'src/main/java/**'Length of output: 435 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo
echo "Searching for 'redundant-hat.md' and occurrences..."
rg -n --hidden --no-ignore -S 'redundant-hat.md' || true
rg -n --hidden --no-ignore -S 'redundant-hat' || true
echo
echo "Searching for motives/errors md files..."
rg -n --hidden --no-ignore -S 'org/eolang/motives/errors' || true
echo
echo "Listing src/main/resources MD files (if present)..."
if [ -d src/main/resources ]; then
find src/main/resources -type f -name '*.md' -print || true
else
echo "src/main/resources not found"
fi
echo
echo "Showing LtRedundantRho.java (first 240 lines)..."
sed -n '1,240p' src/main/java/org/eolang/lints/LtRedundantRho.java || true
echo
echo "Searching repo for motive() usages and tests referencing LtRedundantRho or redundant-hat..."
rg -n --hidden --no-ignore -S 'motive\s*\(|LtRedundantRho|redundant-hat' || trueLength of output: 15067 Create missing motive resource and add a motive test motive() loads "org/eolang/motives/errors/redundant-hat.md" but that file is not present under src/main/resources/org/eolang/motives/errors/ — add src/main/resources/org/eolang/motives/errors/redundant-hat.md (or change the path in src/main/java/org/eolang/lints/LtRedundantRho.java). Ensure the resource is packaged, and add/extend src/test/java/org/eolang/lints/LtRedundantRhoTest.java to assert new LtRedundantRho().motive() returns a non-empty string. 🤖 Prompt for AI Agents |
||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| @Override | ||||||||||||||||||
| public String name() { | ||||||||||||||||||
| return "redundant-hat"; | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2016-2025 Objectionary.com | ||
| * SPDX-License-Identifier: MIT | ||
| */ | ||
| package org.eolang.lints; | ||
|
|
||
| import com.jcabi.xml.XMLDocument; | ||
| import java.io.IOException; | ||
| import org.eolang.parser.EoSyntax; | ||
| import org.hamcrest.MatcherAssert; | ||
| import org.hamcrest.Matchers; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| /** | ||
| * Test for {@link LtRedundantRho}. | ||
| * | ||
| * @since 0.0.1 | ||
| */ | ||
| final class LtRedundantRhoTest { | ||
|
|
||
| @Test | ||
| void reportsNoDefectsForNecessaryHat() throws Exception { | ||
| MatcherAssert.assertThat( | ||
| "Should not report a defect when '^' is necessary to resolve ambiguity", | ||
| new LtRedundantRho().defects( | ||
| new EoSyntax("[] > foo\n+test > @\n^foo > bar").parsed() | ||
| ), | ||
| Matchers.allOf( | ||
| Matchers.iterableWithSize(Matchers.equalTo(0)) | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| void namesStableId() { | ||
| MatcherAssert.assertThat( | ||
| "Rule id must be 'redundant-hat'", | ||
| new LtRedundantRho().name(), | ||
| Matchers.equalTo("redundant-hat") | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| void doesNotReportDefectWhenCaretIsNecessary() throws IOException { | ||
Snehakb1219-christ marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| MatcherAssert.assertThat( | ||
| "Should not report a defect when '^' is necessary due to ambiguity", | ||
| new LtRedundantRho().defects( | ||
| new EoSyntax("[] > foo\n+test > @\n^foo > bar").parsed() | ||
Snehakb1219-christ marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ), | ||
| Matchers.empty() | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| void reportsDefectWhenCaretIsRedundantWithMultipleMatches() throws IOException { | ||
| MatcherAssert.assertThat( | ||
| "Should report a defect when '^' is redundant among multiple matches", | ||
| new LtRedundantRho().defects( | ||
| new XMLDocument("<object><o name='foo' base='ξ.ρ' line='12'></o></object>") | ||
| ), | ||
| Matchers.hasSize(1) | ||
| ); | ||
|
Comment on lines
+55
to
+62
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 🧩 Analysis chainTest name/content mismatch; second branch not covered. The test claims “multiple matches” but provides a single Suggested test data to cover the intended case (adjust if your XMIR differs): - new XMLDocument("<object><o name='foo' base='ξ.ρ' line='12'></o></object>")
+ new XMLDocument(
+ "<object>" +
+ "<o name='foo'/>" +
+ "<o name='inner'>" +
+ "<o name='bar' base='ξ.ρ.foo' line='12'/>" +
+ "</o>" +
+ "</object>"
+ )And add a negative test to prevent false positives when local shadowing exists: @Test
void doesNotReportWhenLocalShadowingRequiresHat() throws IOException {
MatcherAssert.assertThat(
new LtRedundantRho().defects(
new XMLDocument(
"<object>" +
"<o name='foo'/>" +
"<o name='inner'>" +
"<o name='foo'/>" + // local shadow
"<o name='bar' base='ξ.ρ.foo' line='7'/>" +
"</o>" +
"</object>"
)
),
Matchers.emptyIterable()
);
}🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Searching for LtRedundantRhoTest..."
rg -n --hidden -S "LtRedundantRhoTest" || true
echo "---- Showing file (if exists) ----"
sed -n '1,240p' src/test/java/org/eolang/lints/LtRedundantRhoTest.java || true
echo "---- Searching for related test names ----"
rg -n --hidden -S "reportsDefectWhenCaretIsRedundantWithMultipleMatches|doesNotReportWhenLocalShadowingRequiresHat|doesNotReport|reportsDefect" src/test/java/org/eolang/lints || trueLength of output: 2734 Test name/content mismatch — second branch not covered. The test reportsDefectWhenCaretIsRedundantWithMultipleMatches (src/test/java/org/eolang/lints/LtRedundantRhoTest.java:55-62) supplies a single (base='ξ.ρ') and only exercises the single-match branch; rename the test or extend the XML to produce multiple matches. Suggested change: - new XMLDocument("<object><o name='foo' base='ξ.ρ' line='12'></o></object>")
+ new XMLDocument(
+ "<object>" +
+ "<o name='foo'/>" +
+ "<o name='inner'>" +
+ "<o name='bar' base='ξ.ρ.foo' line='12'/>" +
+ "</o>" +
+ "</object>"
+ )Add a negative test to avoid false positives with local shadowing: @Test
void doesNotReportWhenLocalShadowingRequiresHat() throws IOException {
MatcherAssert.assertThat(
new LtRedundantRho().defects(
new XMLDocument(
"<object>" +
"<o name='foo'/>" +
"<o name='inner'>" +
"<o name='foo'/>" + // local shadow
"<o name='bar' base='ξ.ρ.foo' line='7'/>" +
"</o>" +
"</object>"
)
),
Matchers.emptyIterable()
);
}🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.