-
Notifications
You must be signed in to change notification settings - Fork 145
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
#3160 Enable PMD for eo-runime
#3171
Changes from 14 commits
68f7bb9
931958c
22fa0a4
8bf5e84
8e82e95
029b7b2
aa4542d
db2d4d5
cf5db6a
33b264a
ff1c2ed
1286ee0
5f15eb3
058015a
380976e
b2a049c
d3a8376
f4a0dd2
8f827f3
b63ec99
88d82de
2e6fade
56c2ee1
72ce8c8
df95e60
ba2a9e3
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 |
---|---|---|
|
@@ -35,6 +35,7 @@ | |
* @since 0.22 | ||
*/ | ||
@Versionized | ||
@SuppressWarnings({"PMD.TooManyMethods", "PMD.ConstructorShouldDoInitialization"}) | ||
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. @c71n93 I believe, you can easily fix this error (that PMD complains about), instead of suppressing it |
||
final class PhPackage implements Phi { | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,11 +35,13 @@ | |
* @since 0.36 | ||
*/ | ||
@Versionized | ||
@SuppressWarnings("PMD.TooManyMethods") | ||
public final class PhTraced implements Phi { | ||
|
||
/** | ||
* Name of property that responsible for keeping max depth. | ||
*/ | ||
@SuppressWarnings("PMD.LongVariable") | ||
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. @c71n93 why can't you just make the name shorter? 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. @yegor256 Because this variable means "maximum cage recursion depth". As for me the shortest name for this variable is 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. @c71n93 how about |
||
public static final String | ||
MAX_CAGE_RECURSION_DEPTH_PROPERTY_NAME = "EO_MAX_CAGE_RECURSION_DEPTH"; | ||
|
||
|
@@ -58,6 +60,7 @@ public final class PhTraced implements Phi { | |
/** | ||
* Locator of encaged object. | ||
*/ | ||
@SuppressWarnings("PMD.AvoidFieldNameMatchingMethodName") | ||
private final Integer locator; | ||
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. @c71n93 same here, just rename the variable |
||
|
||
/** | ||
|
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. @maxonfjvipon do we need this class? It never used now. 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. @c71n93 we don't need it |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,6 +50,7 @@ public final class PhWrite extends PhDefault implements Atom { | |
* @param attr Attribute name | ||
* @param ret Return value function | ||
*/ | ||
@SuppressWarnings("PMD.ConstructorOnlyInitializesOrCallOtherConstructors") | ||
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. @c71n93 maybe we can rewrite the code instead of suppressing the warning? |
||
public PhWrite( | ||
final String attr, | ||
final Function<Phi, Phi> ret | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,18 +47,17 @@ class SafeFunc<T> implements Supplier<T> { | |
} | ||
|
||
@Override | ||
@SuppressWarnings({"PMD.AvoidCatchingGenericException", "PMD.AvoidRethrowingException"}) | ||
public T get() { | ||
try { | ||
return this.origin.call(); | ||
} catch (final InterruptedException ex) { | ||
Thread.currentThread().interrupt(); | ||
throw new ExInterrupted(); | ||
} catch (final ExAbstract ex) { | ||
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. @c71n93 there was a reason why I added this |
||
throw ex; | ||
throw new ExInterrupted(ex); | ||
// @checkstyle IllegalCatchCheck (3 line) | ||
} catch (final RuntimeException ex) { | ||
throw ex; | ||
} catch (final Throwable ex) { | ||
} catch (final Exception ex) { | ||
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. @c71n93 this seems to be a pretty dangerous change. 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. @yegor256 I made it because of 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. @yegor256 we catch |
||
throw new ExFailure( | ||
String.format( | ||
"Unexpected error '%s' of type %s", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,6 +44,7 @@ public final class VerboseBytesAsString implements Supplier<String> { | |
* Ctor. | ||
* @param data Data. | ||
*/ | ||
@SuppressWarnings("PMD.ArrayIsStoredDirectly") | ||
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. @c71n93 this is a reasonable complaint of PMD, I suggest we fix this |
||
public VerboseBytesAsString(final byte[] data) { | ||
this.data = data; | ||
} | ||
|
@@ -81,6 +82,7 @@ public String get() { | |
Arrays.toString(this.data), | ||
new String(this.data, StandardCharsets.UTF_8) | ||
); | ||
break; | ||
} | ||
return result; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,7 @@ | |
* | ||
* @since 0.16 | ||
*/ | ||
@SuppressWarnings("PMD.JUnit5TestShouldBePackagePrivate") | ||
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. @c71n93 why not just fixing this error? 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. @yegor256 Temporarily this class should be public because it stores public static final String TO_ADD_MESSAGE = "TO ADD ASSERTION MESSAGE"; There is todo about it: https://github.com/c71n93/eo/blob/88b3b4bf5e7cf54efd3a116aac7a41cc35ce54ef/eo-runtime/src/test/java/org/eolang/AtCompositeTest.java#L42. This decision was made here #3113 (review) |
||
public final class AtCompositeTest { | ||
|
||
/** | ||
|
@@ -95,6 +96,7 @@ private static class Rnd extends PhDefault { | |
/** | ||
* Ctor. | ||
*/ | ||
@SuppressWarnings("PMD.ConstructorOnlyInitializesOrCallOtherConstructors") | ||
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. @c71n93 this is also easily fixable 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. @yegor256 As I understand it is common practice to use 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. @c71n93 it's not a brightest design so we should fix it anyway. But it's not a part of this task. It should be done in other PR |
||
Rnd() { | ||
super(); | ||
this.add( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,7 +39,7 @@ | |
* | ||
* @since 0.29 | ||
*/ | ||
class AtLoggedTest { | ||
final class AtLoggedTest { | ||
|
||
/** | ||
* Testable object. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,7 @@ | |
* | ||
* @since 1.0 | ||
*/ | ||
@SuppressWarnings("PMD.TooManyMethods") | ||
final class BytesOfTest { | ||
|
||
@Test | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yegor256 @maxonfjvipon maybe we can move this class to
eo-runtime/src/test/java/EOorg/EOeolang/
? (it uses only there)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@c71n93 I believe it should stay in
org.eolang
since it's kind of part of common runtime object which are used for building the concrete ones