diff --git a/CHANGELOG.md b/CHANGELOG.md index 133b164ab47f..f359e6e71245 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ Compatibility: * Add `rb_gc_mark_locations()` (#3704, @andrykonchin). * Implement `rb_str_format()` (#3716, @andrykonchin). * Add `IO#{pread, pwrite}` methods (#3718, @andrykonchin). +* Emit a warning when `it` call without arguments is used in a block without parameters (#3681, @andrykonchin). Performance: diff --git a/spec/ruby/language/block_spec.rb b/spec/ruby/language/block_spec.rb index cf0931b6884d..a605a3619fdc 100644 --- a/spec/ruby/language/block_spec.rb +++ b/spec/ruby/language/block_spec.rb @@ -1072,3 +1072,32 @@ def all_kwrest(arg1, arg2, *rest, post1, post2, kw1: 1, kw2: 2, okw1:, okw2:, ** end end end + +describe "`it` calls without arguments in a block with no ordinary parameters" do + ruby_version_is "3.3"..."3.4" do + it "emits a deprecation warning" do + -> { + eval "proc { it }" + }.should complain(/warning: `it` calls without arguments will refer to the first block param in Ruby 3.4; use it\(\) or self.it/) + end + + it "does not emit a deprecation warning when a block has parameters" do + -> { eval "proc { |a, b| it }" }.should_not complain + -> { eval "proc { |*rest| it }" }.should_not complain + -> { eval "proc { |*| it }" }.should_not complain + -> { eval "proc { |a:, b:| it }" }.should_not complain + -> { eval "proc { |**kw| it }" }.should_not complain + -> { eval "proc { |**| it }" }.should_not complain + -> { eval "proc { |&block| it }" }.should_not complain + -> { eval "proc { |&| it }" }.should_not complain + end + + it "does not emit a deprecation warning when `it` calls with arguments" do + -> { eval "proc { it(42) }" }.should_not complain + end + + it "does not emit a deprecation warning when `it` calls with explicit empty arguments list" do + -> { eval "proc { it() }" }.should_not complain + end + end +end diff --git a/src/main/java/org/truffleruby/language/methods/Arity.java b/src/main/java/org/truffleruby/language/methods/Arity.java index 9d307c6fa67b..fbfbed504835 100644 --- a/src/main/java/org/truffleruby/language/methods/Arity.java +++ b/src/main/java/org/truffleruby/language/methods/Arity.java @@ -115,6 +115,10 @@ public boolean acceptsKeywords() { return hasKeywords() || hasKeywordsRest(); } + public boolean acceptsPositionalArguments() { + return getRequired() > 0 || getOptional() > 0 || hasRest(); + } + public boolean hasKeywords() { return keywordArguments.length != 0; } diff --git a/src/main/java/org/truffleruby/parser/YARPBlockNodeTranslator.java b/src/main/java/org/truffleruby/parser/YARPBlockNodeTranslator.java index c7b6cb27c9cb..9a710e05f10d 100644 --- a/src/main/java/org/truffleruby/parser/YARPBlockNodeTranslator.java +++ b/src/main/java/org/truffleruby/parser/YARPBlockNodeTranslator.java @@ -43,10 +43,17 @@ public final class YARPBlockNodeTranslator extends YARPTranslator { private final Arity arity; + /** Whether a block itself accepts a block parameter (&block). The Arity class does not contain this information. */ + private final boolean acceptsBlockParameter; - public YARPBlockNodeTranslator(TranslatorEnvironment environment, Arity arity) { - super(environment); + public YARPBlockNodeTranslator( + TranslatorEnvironment environment, + Arity arity, + boolean acceptsBlockParameter, + RubyDeferredWarnings rubyWarnings) { + super(environment, rubyWarnings); this.arity = arity; + this.acceptsBlockParameter = acceptsBlockParameter; } public RubyNode compileBlockNode(Nodes.Node body, Nodes.ParametersNode parameters, String[] locals, @@ -310,4 +317,22 @@ private boolean shouldConsiderDestructuringArrayArg(Arity arity) { } } + @Override + public RubyNode visitCallNode(Nodes.CallNode node) { + // emit a warning if `it` is called without arguments in a block without ordinal parameters + boolean noBlockParameters = !arity.acceptsKeywords() && !arity.acceptsPositionalArguments() && + !acceptsBlockParameter; + if (node.name.equals("it") && node.receiver == null && node.isVariableCall() && noBlockParameters) { + SourceSection section = rubySource.getSource().createSection(node.startOffset, node.length); + + String sourcePath = rubySource.getSourcePath(language).intern(); + int lineNumber = section.getStartLine() + rubySource.getLineOffset(); + String message = "`it` calls without arguments will refer to the first block param in Ruby 3.4; use it() or self.it"; + + rubyWarnings.warn(sourcePath, lineNumber, message); + } + + return super.visitCallNode(node); + } + } diff --git a/src/main/java/org/truffleruby/parser/YARPDefNodeTranslator.java b/src/main/java/org/truffleruby/parser/YARPDefNodeTranslator.java index ee4be80e4226..5ea7c67dd16f 100644 --- a/src/main/java/org/truffleruby/parser/YARPDefNodeTranslator.java +++ b/src/main/java/org/truffleruby/parser/YARPDefNodeTranslator.java @@ -24,8 +24,9 @@ public final class YARPDefNodeTranslator extends YARPTranslator { public YARPDefNodeTranslator( RubyLanguage language, - TranslatorEnvironment environment) { - super(environment); + TranslatorEnvironment environment, + RubyDeferredWarnings rubyWarnings) { + super(environment, rubyWarnings); if (parseEnvironment.parserContext.isEval() || parseEnvironment.isCoverageEnabled()) { shouldLazyTranslate = false; diff --git a/src/main/java/org/truffleruby/parser/YARPTranslator.java b/src/main/java/org/truffleruby/parser/YARPTranslator.java index fcbf0718f62f..e05b70ac874c 100644 --- a/src/main/java/org/truffleruby/parser/YARPTranslator.java +++ b/src/main/java/org/truffleruby/parser/YARPTranslator.java @@ -175,6 +175,9 @@ public class YARPTranslator extends YARPBaseTranslator { public static final RescueNode[] EMPTY_RESCUE_NODE_ARRAY = new RescueNode[0]; public Deque frameOnStackMarkerSlotStack = new ArrayDeque<>(); + + protected RubyDeferredWarnings rubyWarnings; + /** whether a while-loop body is translated; needed to check correctness of operators like break/next/etc */ private boolean translatingWhile = false; /** whether a for-loop body is translated; needed to enforce variables in the for-loop body to be declared outside @@ -198,8 +201,9 @@ public class YARPTranslator extends YARPBaseTranslator { /** all the encountered BEGIN {} blocks; they will be added finally at the beginning of the program AST */ private final ArrayList beginBlocks = new ArrayList<>(); - public YARPTranslator(TranslatorEnvironment environment) { + public YARPTranslator(TranslatorEnvironment environment, RubyDeferredWarnings rubyWarnings) { super(environment); + this.rubyWarnings = rubyWarnings; } public RubyNode[] getBeginBlocks() { @@ -537,7 +541,9 @@ private RubyNode translateBlockAndLambda(Nodes.Node node, Nodes.Node parametersN final YARPBlockNodeTranslator methodCompiler = new YARPBlockNodeTranslator( newEnvironment, - arity); + arity, + parameters.block != null, + rubyWarnings); methodCompiler.frameOnStackMarkerSlotStack = frameOnStackMarkerSlotStack; @@ -1548,7 +1554,8 @@ public RubyNode visitDefNode(Nodes.DefNode node) { final var defNodeTranslator = new YARPDefNodeTranslator( language, - newEnvironment); + newEnvironment, + rubyWarnings); var callTargetSupplier = defNodeTranslator.buildMethodNodeCompiler(node, parameters, arity); final boolean isDefSingleton = singletonClassNode != null; @@ -3562,7 +3569,7 @@ private RubyNode openModule(Nodes.Node moduleNode, RubyNode defineOrGetNode, Str newEnvironment.declareVar(name); } - final YARPTranslator moduleTranslator = new YARPTranslator(newEnvironment); + final YARPTranslator moduleTranslator = new YARPTranslator(newEnvironment, rubyWarnings); final ModuleBodyDefinition definition = moduleTranslator.compileClassNode(moduleNode, bodyNode); return new RunModuleDefinitionNode(definition, defineOrGetNode); } diff --git a/src/main/java/org/truffleruby/parser/YARPTranslatorDriver.java b/src/main/java/org/truffleruby/parser/YARPTranslatorDriver.java index 604fee6115c7..911dc0cdaba2 100644 --- a/src/main/java/org/truffleruby/parser/YARPTranslatorDriver.java +++ b/src/main/java/org/truffleruby/parser/YARPTranslatorDriver.java @@ -222,7 +222,7 @@ public RootCallTarget parse(RubySource rubySource, ParserContext parserContext, // Translate to Ruby Truffle nodes // use source encoding detected by manually, before source file is fully parsed - final YARPTranslator translator = new YARPTranslator(environment); + final YARPTranslator translator = new YARPTranslator(environment, rubyWarnings); RubyNode truffleNode; printParseTranslateExecuteMetric("before-translate", context, source);