Skip to content

[feat] make Ruby#setWarningsEnabled actually useful #7728

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

Merged
kares merged 13 commits into from
Apr 5, 2023
Merged

[feat] make Ruby#setWarningsEnabled actually useful #7728

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
26b616a
[feat] make setWarningsEnabled useful (for embed)
kares Mar 15, 2023
5dc9c62
[refactor] proper category - used with deprecated methods
kares Mar 16, 2023
d426513
[refactor] no need to check verbose - warningsEnabled does check
kares Mar 16, 2023
e883ffd
[refactor] set $VERBOSE and $DEBUG globals once
kares Mar 16, 2023
51c1ebf
[refactor] route warnings through one (doWarn) method
kares Mar 18, 2023
80c2cc5
[refactor] unify "given block not used" warning dispatch
kares Mar 20, 2023
60d4cdb
[feat] REDEFINING_METHOD warning + tag deprecated warn
kares Mar 18, 2023
f2a36a5
[refactor] fix (0-based) parser/lexer line numbers at the source
kares Mar 18, 2023
4f4de1a
[refactor] dry-out warnings further + remove redundant line--
kares Mar 20, 2023
947c8f3
[refactor] fix (0-based) parser/lexer line numbers at the source
kares Mar 20, 2023
079b7ae
[feat] introduce a "deprecated block" warning identifier
kares Mar 21, 2023
3c329b6
[refactor] review warnings usage in RubyModule
kares Mar 21, 2023
45b1dbd
[test] line numbers reported are correct (#7741)
kares Apr 4, 2023
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
44 changes: 34 additions & 10 deletions core/src/main/java/org/jruby/Ruby.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2515,19 +2515,29 @@ public RubyHash getCharsetMap() {
return charsetMap;
}

/** Getter for property isVerbose.
* @return Value of property isVerbose.
/**
* @return $VERBOSE value
*/
public IRubyObject getVerbose() {
return verboseValue;
}

/**
* @return $VERBOSE value as a Java primitive
*/
public boolean isVerbose() {
return verbose;
}

/**
* If the user explicitly disabled warnings using: {@link #setWarningsEnabled(boolean)} return false.
*
* Otherwise fallback to a $VERBOSE value check (which is the default behavior).
*
* @return whether warnings are enabled
*/
public boolean warningsEnabled() {
return warningsEnabled;
return warningsEnabled && verboseWarnings;
}

/**
Expand All @@ -2547,7 +2557,7 @@ public void setWarningsEnabled(final boolean warningsEnabled) {
public void setVerbose(final IRubyObject verbose) {
this.verbose = verbose.isTrue();
this.verboseValue = verbose;
warningsEnabled = !verbose.isNil();
verboseWarnings = !verbose.isNil();
}

/**
Expand All @@ -2559,22 +2569,34 @@ public void setVerbose(final Boolean verbose) {
setVerbose(verbose == null ? nilObject : (verbose ? trueObject : falseObject));
}

/** Getter for property isDebug.
* @return Value of property isDebug.
/**
* @return $DEBUG value
*/
public IRubyObject getDebug() {
return debug ? trueObject : falseObject;
}

/**
* @return $DEBUG value as a boolean
*/
public boolean isDebug() {
return debug;
}

/** Setter for property isDebug.
* @param debug New value of property isDebug.
/**
* Setter for property isDebug.
* @param debug the $DEBUG value
*/
public void setDebug(IRubyObject debug) {
this.debug = debug.isTrue();
setDebug(debug.isTrue());
}

/**
* Sets the $DEBUG flag
* @param debug
*/
public void setDebug(final boolean debug) {
this.debug = debug;
}

/**
Expand Down Expand Up @@ -5266,7 +5288,9 @@ public RubyClass getData() {
@Deprecated
private IRubyObject rootFiber;

private boolean verbose, warningsEnabled, debug;
private boolean warningsEnabled = true; // global flag to be able to disable warnings regardless of $VERBOSE
private boolean verboseWarnings; // whether warnings are enabled based on $VERBOSE
private boolean verbose, debug;
private IRubyObject verboseValue;

// Set of categories we care about (set defined when creating warnings).
Expand Down
14 changes: 7 additions & 7 deletions core/src/main/java/org/jruby/RubyArray.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2548,7 +2548,7 @@ public IRubyObject index(ThreadContext context, IRubyObject obj) {

@JRubyMethod(name = {"index", "find_index"})
public IRubyObject index(ThreadContext context, IRubyObject obj, Block unused) {
if (unused.isGiven()) context.runtime.getWarnings().warn(ID.BLOCK_UNUSED, "given block not used");
if (unused.isGiven()) context.runtime.getWarnings().warning(ID.BLOCK_UNUSED, "given block not used");
return index(context, obj);
}

Expand Down Expand Up @@ -2655,7 +2655,7 @@ public IRubyObject rindex(ThreadContext context, IRubyObject obj) {

@JRubyMethod
public IRubyObject rindex(ThreadContext context, IRubyObject obj, Block unused) {
if (unused.isGiven()) context.runtime.getWarnings().warn(ID.BLOCK_UNUSED, "given block not used");
if (unused.isGiven()) context.runtime.getWarnings().warning(ID.BLOCK_UNUSED, "given block not used");
return rindex(context, obj);
}

Expand Down Expand Up @@ -3519,7 +3519,7 @@ public IRubyObject count(ThreadContext context, Block block) {

@JRubyMethod(name = "count")
public IRubyObject count(ThreadContext context, IRubyObject obj, Block block) {
if (block.isGiven()) context.runtime.getWarnings().warn(ID.BLOCK_UNUSED, "given block not used");
if (block.isGiven()) context.runtime.getWarnings().warning(ID.BLOCK_UNUSED, "given block not used");

int n = 0;
for (int i = 0; i < realLength; i++) {
Expand Down Expand Up @@ -4927,7 +4927,7 @@ public IRubyObject all_pCommon(ThreadContext context, IRubyObject arg, Block blo
boolean patternGiven = arg != null;

if (block.isGiven() && patternGiven) {
context.runtime.getWarnings().warn("given block not used");
context.runtime.getWarnings().warning(ID.BLOCK_UNUSED, "given block not used");
}

if (!block.isGiven() || patternGiven) return all_pBlockless(context, arg);
Expand Down Expand Up @@ -4970,7 +4970,7 @@ public IRubyObject any_pCommon(ThreadContext context, IRubyObject arg, Block blo
boolean patternGiven = arg != null;

if (block.isGiven() && patternGiven) {
context.runtime.getWarnings().warn("given block not used");
context.runtime.getWarnings().warning(ID.BLOCK_UNUSED, "given block not used");
}

if (!block.isGiven() || patternGiven) return any_pBlockless(context, arg);
Expand Down Expand Up @@ -5012,7 +5012,7 @@ public IRubyObject none_pCommon(ThreadContext context, IRubyObject arg, Block bl
boolean patternGiven = arg != null;

if (block.isGiven() && patternGiven) {
context.runtime.getWarnings().warn("given block not used");
context.runtime.getWarnings().warning(ID.BLOCK_UNUSED, "given block not used");
}

if (!block.isGiven() || patternGiven) return none_pBlockless(context, arg);
Expand Down Expand Up @@ -5054,7 +5054,7 @@ public IRubyObject one_pCommon(ThreadContext context, IRubyObject arg, Block blo
boolean patternGiven = arg != null;

if (block.isGiven() && patternGiven) {
context.runtime.getWarnings().warn("given block not used");
context.runtime.getWarnings().warning(ID.BLOCK_UNUSED, "given block not used");
}

if (!block.isGiven() || patternGiven) return one_pBlockless(context, arg);
Expand Down
14 changes: 7 additions & 7 deletions core/src/main/java/org/jruby/RubyEnumerable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ public static IRubyObject count(ThreadContext context, IRubyObject self, final I
final Ruby runtime = context.runtime;
final SingleInt result = new SingleInt();

if (block.isGiven()) runtime.getWarnings().warn(ID.BLOCK_UNUSED , "given block not used");
if (block.isGiven()) runtime.getWarnings().warning(ID.BLOCK_UNUSED , "given block not used");

each(context, eachSite(context), self, new JavaInternalBlockBody(runtime, context, "Enumerable#count", Signature.ONE_REQUIRED) {
public IRubyObject yield(ThreadContext context1, IRubyObject[] args) {
Expand Down Expand Up @@ -707,7 +707,7 @@ public static IRubyObject find_index(ThreadContext context, IRubyObject self, fi
public static IRubyObject find_index(ThreadContext context, IRubyObject self, final IRubyObject cond, final Block block) {
final Ruby runtime = context.runtime;

if (block.isGiven()) runtime.getWarnings().warn(ID.BLOCK_UNUSED , "given block not used");
if (block.isGiven()) runtime.getWarnings().warning(ID.BLOCK_UNUSED , "given block not used");
if (self instanceof RubyArray) return ((RubyArray) self).find_index(context, cond);

return find_indexCommon(context, eachSite(context), self, cond);
Expand Down Expand Up @@ -1103,7 +1103,7 @@ public static IRubyObject inject(ThreadContext context, IRubyObject self, IRubyO
public static IRubyObject inject(ThreadContext context, IRubyObject self, IRubyObject init, IRubyObject method, final Block block) {
final Ruby runtime = context.runtime;

if (block.isGiven()) runtime.getWarnings().warn(ID.BLOCK_UNUSED , "given block not used");
if (block.isGiven()) runtime.getWarnings().warning(ID.BLOCK_UNUSED , "given block not used");

final String methodId = method.asJavaString();
final SingleObject<IRubyObject> result = new SingleObject<>(init);
Expand Down Expand Up @@ -1608,7 +1608,7 @@ public static IRubyObject none_pCommon(ThreadContext context, CallSite each, IRu
final boolean patternGiven = pattern != null;

if (block.isGiven() && patternGiven) {
context.runtime.getWarnings().warn("given block not used");
context.runtime.getWarnings().warning(ID.BLOCK_UNUSED, "given block not used");
}

try {
Expand Down Expand Up @@ -1663,7 +1663,7 @@ public static IRubyObject one_pCommon(ThreadContext context, CallSite each, IRub
final boolean patternGiven = pattern != null;

if (block.isGiven() && patternGiven) {
context.runtime.getWarnings().warn("given block not used");
context.runtime.getWarnings().warning(ID.BLOCK_UNUSED, "given block not used");
}

try {
Expand Down Expand Up @@ -1746,7 +1746,7 @@ public static IRubyObject all_pCommon(ThreadContext localContext, CallSite each,
final boolean patternGiven = pattern != null;

if (block.isGiven() && patternGiven) {
localContext.runtime.getWarnings().warn("given block not used");
localContext.runtime.getWarnings().warning(ID.BLOCK_UNUSED, "given block not used");
}

try {
Expand Down Expand Up @@ -1836,7 +1836,7 @@ public static IRubyObject any_pCommon(ThreadContext localContext, CallSite site,
final boolean patternGiven = pattern != null;

if (block.isGiven() && patternGiven) {
localContext.runtime.getWarnings().warn("given block not used");
localContext.runtime.getWarnings().warning(ID.BLOCK_UNUSED, "given block not used");
}

try {
Expand Down
35 changes: 13 additions & 22 deletions core/src/main/java/org/jruby/RubyGlobal.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,13 +198,14 @@ public static void createGlobals(Ruby runtime) {
} else {
verboseValue = runtime.getFalse();
}
runtime.defineVariable(new VerboseGlobalVariable(runtime, "$VERBOSE", verboseValue), GLOBAL);
runtime.defineVariable(new VerboseGlobalVariable(runtime, "$-v", verboseValue), GLOBAL);
runtime.defineVariable(new VerboseGlobalVariable(runtime, "$-w", verboseValue), GLOBAL);
runtime.setVerbose(verboseValue);
runtime.defineVariable(new VerboseGlobalVariable(runtime, "$VERBOSE"), GLOBAL);
runtime.defineVariable(new VerboseGlobalVariable(runtime, "$-v"), GLOBAL);
runtime.defineVariable(new VerboseGlobalVariable(runtime, "$-w"), GLOBAL);

IRubyObject debug = runtime.newBoolean(runtime.getInstanceConfig().isDebug());
runtime.defineVariable(new DebugGlobalVariable(runtime, "$DEBUG", debug), GLOBAL);
runtime.defineVariable(new DebugGlobalVariable(runtime, "$-d", debug), GLOBAL);
runtime.setDebug(runtime.newBoolean(runtime.getInstanceConfig().isDebug()));
runtime.defineVariable(new DebugGlobalVariable(runtime, "$DEBUG"), GLOBAL);
runtime.defineVariable(new DebugGlobalVariable(runtime, "$-d"), GLOBAL);

runtime.defineVariable(new BacktraceGlobalVariable(runtime, "$@"), THREAD);

Expand Down Expand Up @@ -1061,9 +1062,8 @@ public IRubyObject set(IRubyObject value) {
}

private static class VerboseGlobalVariable extends GlobalVariable {
public VerboseGlobalVariable(Ruby runtime, String name, IRubyObject initialValue) {
super(runtime, name, initialValue);
set(initialValue);
public VerboseGlobalVariable(Ruby runtime, String name) {
super(runtime, name, null); // this.value not used
}

@Override
Expand All @@ -1073,11 +1073,7 @@ public IRubyObject get() {

@Override
public IRubyObject set(IRubyObject newValue) {
if (newValue.isNil()) {
runtime.setVerbose(newValue);
} else {
runtime.setVerbose(runtime.newBoolean(newValue.isTrue()));
}
runtime.setVerbose(newValue.isNil() ? null : newValue.isTrue());

return newValue;
}
Expand All @@ -1095,9 +1091,8 @@ public WarningGlobalVariable(Ruby runtime, String name, RubyInstanceConfig.Verbo
}

private static class DebugGlobalVariable extends GlobalVariable {
public DebugGlobalVariable(Ruby runtime, String name, IRubyObject initialValue) {
super(runtime, name, initialValue);
set(initialValue);
public DebugGlobalVariable(Ruby runtime, String name) {
super(runtime, name, null); // this.value not used
}

@Override
Expand All @@ -1107,11 +1102,7 @@ public IRubyObject get() {

@Override
public IRubyObject set(IRubyObject newValue) {
if (newValue.isNil()) {
runtime.setDebug(newValue);
} else {
runtime.setDebug(runtime.newBoolean(newValue.isTrue()));
}
runtime.setDebug(newValue.isTrue());

return newValue;
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/RubyKernel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1375,7 +1375,7 @@ static void warnStr(ThreadContext context, IRubyObject recv, RubyString message,
}

if (recv == runtime.getWarning()) {
RubyWarnings.warn(context, recv, message);
RubyWarnings.warn(context, message);
return;
}

Expand Down
15 changes: 7 additions & 8 deletions core/src/main/java/org/jruby/RubyModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@
import org.jruby.anno.JRubyMethod;
import org.jruby.anno.JavaMethodDescriptor;
import org.jruby.anno.TypePopulator;
import org.jruby.common.IRubyWarnings;
import org.jruby.common.IRubyWarnings.ID;
import org.jruby.embed.Extension;
import org.jruby.exceptions.LoadError;
Expand Down Expand Up @@ -2064,11 +2063,11 @@ public synchronized void defineAlias(String name, String oldName) {
DynamicMethod method = getMethods().get(name);
if (method != null && entry.method.getRealMethod() != method.getRealMethod() && !method.isUndefined()) {
// warn if overwriting an existing method on this module
if (method.getRealMethod().getAliasCount() == 0) runtime.getWarnings().warning("method redefined; discarding old " + name);
if (method.getRealMethod().getAliasCount() == 0) runtime.getWarnings().warning(ID.REDEFINING_METHOD, "method redefined; discarding old " + name);

if (method instanceof PositionAware) {
PositionAware posAware = (PositionAware) method;
runtime.getWarnings().warning(posAware.getFile(), posAware.getLine(), "previous definition of bar " + name);
runtime.getWarnings().warning(ID.REDEFINING_METHOD, posAware.getFile(), posAware.getLine() + 1, "previous definition of " + name + " was here");
}
}

Expand Down Expand Up @@ -2983,13 +2982,13 @@ public IRubyObject ruby2_keywords(ThreadContext context, IRubyObject[] args) {
if (signature.hasRest() && !signature.hasKwargs()) {
method.setRuby2Keywords();
} else {
context.runtime.getWarnings().warn(IRubyWarnings.ID.MISCELLANEOUS, str(context.runtime, "Skipping set of ruby2_keywords flag for ", name, " (method accepts keywords or method does not accept argument splat)"));
context.runtime.getWarnings().warn(ID.MISCELLANEOUS, str(context.runtime, "Skipping set of ruby2_keywords flag for ", name, " (method accepts keywords or method does not accept argument splat)"));
}
} else {
context.runtime.getWarnings().warn(IRubyWarnings.ID.MISCELLANEOUS, str(context.runtime, "Skipping set of ruby2_keywords flag for ", name, " (method not defined in Ruby)"));
context.runtime.getWarnings().warn(ID.MISCELLANEOUS, str(context.runtime, "Skipping set of ruby2_keywords flag for ", name, " (method not defined in Ruby)"));
}
} else {
context.runtime.getWarnings().warn(IRubyWarnings.ID.MISCELLANEOUS, str(context.runtime, "Skipping set of ruby2_keywords flag for ", name, " (can only set in method defining module)"));
context.runtime.getWarnings().warn(ID.MISCELLANEOUS, str(context.runtime, "Skipping set of ruby2_keywords flag for ", name, " (can only set in method defining module)"));
}
}
return context.nil;
Expand Down Expand Up @@ -4903,9 +4902,9 @@ private IRubyObject setConstantCommon(String name, IRubyObject value, boolean hi
if (notAutoload || !setAutoloadConstant(name, value, file, line)) {
if (warn && notAutoload) {
if (this.equals(getRuntime().getObject())) {
getRuntime().getWarnings().warn(ID.CONSTANT_ALREADY_INITIALIZED, "already initialized constant " + name);
getRuntime().getWarnings().warning(ID.CONSTANT_ALREADY_INITIALIZED, "already initialized constant " + name);
} else {
getRuntime().getWarnings().warn(ID.CONSTANT_ALREADY_INITIALIZED, "already initialized constant " + this + "::" + name);
getRuntime().getWarnings().warning(ID.CONSTANT_ALREADY_INITIALIZED, "already initialized constant " + this + "::" + name);
}
}

Expand Down
Loading