rubocop / ruby-style-guide

A community-driven Ruby coding style guide
https://rubystyle.guide
16.43k stars 3.4k forks source link

Methods that have "keyword" status? #559

Closed hediyi closed 4 years ago

hediyi commented 8 years ago

In the rule that governs the use of parentheses in method invocations, the part

Omit parentheses around parameters for ... methods that have "keyword" status in Ruby (e.g. attr_reader, puts)

isn't clear, especially for people new to Ruby, as to what exactly is "keyword" status in Ruby, or which methods.

I think we should elaborate on it to make it instructive enough to be useful : )

dsounded commented 8 years ago

@esdoppio I've opened PR for this, check this out:

https://github.com/bbatsov/ruby-style-guide/pull/554/files

hediyi commented 8 years ago

Still don't think the "keyword" part is really instructive. It's like you have a vague feeling about which methods have "keyword" status but there isn't a real consensus here. So I don't think it can be justified as part of the rule.

lucasrodcosta commented 8 years ago

Totally agree with @esdoppio.

The expression "methods with keyword status" don't have a strict definition and isn't a consensus. A Google search for expressions like "ruby keyword methods", "ruby keyword status methods" and similars returns results related to "keywords arguments" introduced in Ruby 2.0.

For people new in Ruby, this might be confusing...

hediyi commented 8 years ago

@lucasrodcosta yeah, for a token, it's either a keyword, or it's not. There's no such thing as a method that has "keyword" status. I think the rule is probably trying to say "methods that could be keywords in other programming languages," e.g., print, exit, and require.

I want to open a PR for this. @bbatsov what do you think?

lucasrodcosta commented 8 years ago

I think that reference to other programming languages isn't ideal, because this guide should be self-contained (that is, we shouldn't assume that the reader must know the keywords lists of other programming languages to understand the Ruby Style Guide).

Particularly, I like the rules used in the Airbnb's Ruby Style Guide.

Use parentheses for a method call if the method returns a value and if the first argument to the method uses parentheses. Omit parentheses for a method call if the method accepts no arguments. If the method doesn't return a value (or we don't care about the return), parentheses are optional.

But we may adapt the last rule in order to remove the "optional" term and make the rule less ambiguous. What do you think?

hediyi commented 8 years ago

@lucasrodcosta that's exactly the style guide I read when I was learning ruby.

"Could be a keyword," that's just what the rule in this guide implies, which is why I think it's not a good rule. I was collecting samples, and trying to make a summary of which methods (methods that are not part of a DSL, and require arguments) are usually called without parentheses.

doesn't return a value (or we don't care about the return)

seems like a good fit, at least it's better than this "keyword" rule.

DanRathbun commented 6 years ago

@esdoppio I think the rule is probably trying to say "methods that could be keywords in other programming languages," e.g., print, exit, and require.

NO it is not trying to say this. The guideline (rule) is in fact using the incorrect term "keyword status", when it should be saying "reserved word status".

And I believe I saw somewhere in a well regarded source (which I cannot remember which) that the "reserved word" methods are global methods from module Kernel and classes Object and BasicObject. Some people say that any method that is global such as DSL methods mixed into the toplevel, have "reserved word status". (This style guide for an example.)

@esdoppio There's no such thing as a method that has "keyword" status.

True because the wrong term was used. The use of the term "keyword" as a synonym for "reserved word" predated the implementation of ...

@lucasrodcosta A Google search ... returns results related to "keywords arguments" introduced in Ruby 2.0.

Remember that Ruby 2+ keyword arguments are automatically collected into a hash using the keywords as hash keys. So the Ruby 2+ term "keyword" is correctly applied, whereas the old practice of using "keyword" as a synonym for "reserved word" comes from general (and IMO "sloppy") use.

We can find many references on the web to programming languages having "keywords", but reading the definition, we can see this is not the correct term for "reserved words".


Anyway, ... in conclusion, if :

(a) the two references in the style guide are changed from "keyword status", to "reserved word status", then the confusion (with method keyword arguments) would not happen, ...

(b) and mention is added to define the global methods from module Kernel and classes Object and BasicObject as having "reserved word status", ...

... then this issue could be closed.

bbatsov commented 5 years ago

PRs improving the current wording would be most welcome!

jlmuir commented 4 years ago

I think the crux of this issue is that it's not clear which methods have "keyword status." If that could be answered, I'd be happy to submit a PR. So, which methods have keyword status?

I could guess for the DSL methods:

Are there more? If so, which ones?

And for the non-DSL methods, I would guess the following Kernel methods:

Are there more? If so, which ones?

Or is it futile to attempt to clearly list the keyword-status methods because "keyword status" is very loosely defined?

If it's loosely defined, then I could submit a PR that would basically say that it's considered good style to not use parentheses with arguments for non-DSL methods that are considered to have keyword status, but that the list of those methods is not well-defined, so you have to use your best judgement. And it would say that another good style that is used by some which is well-defined and consistent is to always use parentheses with arguments for non-DSL methods, even the ones that some people consider to have keyword status, e.g.:

bbatsov commented 4 years ago

Are there more? If so, which ones?

There are also many 3rd party methods - e.g. all of Rails's model validators, but we should just give a few examples not list every possible macro-style method.

If it's loosely defined, then I could submit a PR that would basically say that it's considered good style to not use parentheses with arguments for non-DSL methods that are considered to have keyword status, but that the list of those methods is not well-defined, so you have to use your best judgement. And it would say that another good style that is used by some which is well-defined and consistent is to always use parentheses with arguments for non-DSL methods, even the ones that some people consider to have keyword status, e.g.:

I like this approach. Generally I don't really get why methods like puts got their "special" status given their pretty ordinary semantics.

DanRathbun commented 4 years ago

@jlmuir: I think the crux of this issue is that it's not clear which methods have "keyword status." ... Or is it futile to attempt to clearly list the keyword-status methods because "keyword status" is very loosely defined?

As I explained above (almost 2 years ago,) the term "keyword status" is incorrect and needs to be changed (as part of any PR,) to "reserved word status."

@jlmuir: ... instance methods with official Ruby API documentation examples that show them being used with keyword-status style [i.e., with arguments and without parentheses]?

Historical usage in Ruby Core docs should not be a criteria. Not only are the doc examples without consistency, but many of the old pure Ruby files in the Standard Library lack proper use of parenthesis.

The docs can give clues to good use and bad use.

@jlmuir: If it's loosely defined, ...

I loosely defined it above (almost 2 years ago,) as ...

@DanRathbun: (b) and mention is added to define the global methods from module Kernel and classes Object and BasicObject as having "reserved word status", ...

... however this doesn't mean such methods use are required nor suggested to omit parenthesis, ... it means that the omission of parenthesis are allowed as readability warrants.

As an example, I'll use the global puts method without parenthesis only if there is 1 argument. If there are multiple arguments, I'll always use parenthesis.

@jlmuir: ... then I could submit a PR that would basically say that it's considered good style to not use parentheses with arguments for non-DSL methods that are considered to have keyword status ...

This (emboldened above) is a bit butt-backward. The general rule should be to encourage the use of parenthesized argument lists whenever it favors readability, and also not to use parenthesis when their use would take away from readability.

A couple of prime examples for not (using parenthesis) is operator method calls, such as:

if index < 1 # as opposed to index.<(1)

... or ...

"%-5s: %08x" % [ "ID", self.object_id ] # and not: "%-5s: %08x".%([ "ID", self.object_id ])

The use of method calls without parenthesis used to generate a "parenthesize arguments for future compatibility" warning. This implied that in a future version method calls would force the use of parenthesized arguments.

As a result, I've long gotten into the habit of always using parenthesis in instance methods calls from outside an instance object, (ie, whenever using receiver.meth() notation,) regardless of argument count. I think this should be a style "suggestion" itself, call it (c).

@jlmuir: I could guess for the DSL methods: ... (all Module instance methods ...)

IMO, Core class Module and class Class instance methods are not Domain Specific Language methods. They are uh ... language core, ... fundamental to the classes.

But this does bring up an interesting use as another "rule", call it (d).

This would specifically apply to instance method calls from inside class or module definitions. When the receiver is self. or the implied self (and not specified,) then parenthesis are allowed to be omitted, if readability warrants.

This means that they can be used at the coder's discretion. For example, within a module (or class) ...

attr(:name, :description, :color, :height, :width)

... is fine and should not generate a warning or "break a style rule" for using parenthesis.

Nor should this raise any warning ...

attr :name, :description, :color, :height, :width

There are also many 3rd party methods - e.g. all of Rails's model validators, but we should just give a few examples not list every possible macro-style method.

Rails should define their own forked style guide. This repo should apply to plain Ruby so it can be adopted by any DSL environment without polluting them with some other DSL's "rules".

But, generally, I would have to say that we could encourage the "idea" that a certain set of DSL methods could enjoy "reserved word status", if they mixed their methods (say via library mixin modules) into module Kernel or classes BasicObject, Object, Module or Class so as to make these DSL methods global.

jlmuir commented 4 years ago

As I explained above (almost 2 years ago,) the term "keyword status" is incorrect and needs to be changed (as part of any PR,) to "reserved word status."

@DanRathbun: I don't follow. The official Ruby API documentation still has a Keywords document, and it calls them keywords.

DanRathbun commented 4 years ago

@jlmuir The "official documentation" you refer to ... predated the implementation of keyword arguments.

Why would you assume that the core documentation is without error? This only means that a change is in order for that document. (The heading in the document body should change, but the could be changed as well.)

Dave Thomas and Andrew Hunt were an authority on Ruby long before you or I. In every edition of their "Pick Axe" book they've included a table labeled "Reserved words".

From the "Programming Ruby - The Pragmatic Programmer's Guide" (4th edition) by Dave Thomas, Andrew Hunt (some editions with Chad Fowler) Part III. - Ruby Crystallized, Chapter 22. The Ruby Language

22.4 Names

Ruby names are used to refer to constants, variables, methods, classes, and modules. The first character of a name helps Ruby distinguish its intended use. Certain names, listed in the following table, are reserved words and should not be used as variable, method, class, or module names. __ENCODING__ __FILE__ __LINE__ BEGIN END alias and begin
break case class def defined? do else elsif end
ensure false for if in module next nil not
or redo rescue retry return self super then true
undef unless until when while yield

Table 12—Reserved words

And more so, that ruby-doc.org (the official Ruby language documentation site) has posted the entire 1st edition of this "Pick Axe" book if you wish to see for yourself. This book was written with close communication with Yukihiro Matsumoto himself. ("Matz" even wrote the forwards for all the books.) ... http://ruby-doc.com/docs/ProgrammingRuby/html/language.html#S3


Beyond what docs and books say, ... the term "keyword" is not accurate to describe the reserved words of a programming language. A "keyword" is something used to unlock a cypher or encrypted archive. Or to look up something in a database (or data collection of some kind.)

In Ruby, the use of "keyword" could also cause confusion with the use of hash keys.

Some person in the past IMO, got lazy and stole the term to apply to reserved words in programming languages. Listen, I know that it has become a commonplace convention to use the term "keyword". I am arguing that the convention is poor practice. It seems that Dave Thomas thought so as well. He purposely labeled his table Reserved words and not Keywords.

He published his authoritative book in (2000) the early 1.6 trunk. The "official" keywords.rdoc file was not created until the 1.9.1 release (at least 9 years later.)

jlmuir commented 4 years ago

Why would you assume that the core documentation is without error? This only means that a change is in order for that document. (The heading in the document body should change, but the could be changed as well.)

@DanRathbun: Why would I not? I expect the official documentation of any project to be authoritative unless the authors have explicitly delegated authority to a book or some other reference material.

I guess Ruby is defined as an ISO standard in ISO/IEC 30170:2012, but you have to buy it to read it. I'd be curious to know what it calls them.

Beyond what docs and books say, ... the term "keyword" is not accurate to describe the reserved words of a programming language. A "keyword" is something used to unlock a cypher or encrypted archive. Or to look up something in a database (or data collection of some kind.)

I don't think so. Microsoft uses the term "keywords" in C Keywords, there's more talk of keywords in the Variables and types section of cplusplus.com's C++ Language tutorial, the Lua reference manual talks about keywords in the The Language chapter, and the Oracle Java Tutorials collection has a Java Language Keywords section, to name a few.

I'd be happy to switch to calling them reserved words if it can be shown that that is the preferred name for them, but right now I'm not seeing much evidence except that the Programming Ruby book refers to them that way. I do like that book, BTW, and I respect the authors. (I have only read parts of the edition that is freely available online, though.) If you could get it changed in the official Ruby API documentation, then, for whatever it's worth, I'd support changing it in this style guide.

jlmuir commented 4 years ago

They also seem to be referred to as keywords in the MRI/CRuby source code at ruby/defs/keywords.

DanRathbun commented 4 years ago

@jlmuir : They also seem to be referred to as keywords in the MRI/CRuby source code at ruby/defs/keywords.

I quote lines 1 through 7 of that file ...

%{
struct kwtable {short name, id[2], state;};
const struct kwtable *rb_reserved_word(const char *, unsigned int);
#ifndef RIPPER
static const struct kwtable *reserved_word(/*!ANSI{*/const char *, unsigned int/*}!ANSI*/);
#define rb_reserved_word(str, len) reserved_word(str, len)
%}

Note the defining of the reserved_word() macro for the rb_reserved_word() function (from source in "lex.c".)

I understand that in any programming language the parsing code can make use of the term keyword and it will make sense to those writing and maintaining the parser because it's a lookup.

But this is an internal implementation detail that need not be exposed to the end user (in this case the non-C literate Ruby scriptwriters.)


@jlmuir : I don't think so. Microsoft uses the term "keywords" in C Keywords, there's more talk of keywords in the Variables and types section of cplusplus.com's C++ Language tutorial, the Lua reference manual talks about keywords in the The Language chapter, and the Oracle Java Tutorials collection has a Java Language Keywords section, to name a few.

It seems this debate has been going on for some time ...

https://en.wikipedia.org/wiki/Reserved_word

Reserved word

In a computer language, a reserved word (also known as a reserved identifier) is a word that cannot be used as an identifier, such as the name of a variable, function, or label – it is "reserved from use". This is a syntactic definition, and a reserved word may have no meaning.

A closely related and often conflated notion is a keyword, which is a word with special meaning in a particular context. This is a semantic definition. By contrast, names in a standard library but not built into the language are not considered reserved words or keywords. The terms "reserved word" and "keyword" are often used interchangeably – one may say that a reserved word is "reserved for use as a keyword" – and formal use varies from language to language; for this article we distinguish as above.

In general reserved words and keywords need not coincide, but in most modern languages keywords are a subset of reserved words, as this makes parsing easier, since keywords cannot be confused with identifiers. In some languages, like C or Python, reserved words and keywords coincide, while in other languages, like Java, all keywords are reserved words, but some reserved words are not keywords – these are "reserved for future use". In yet other languages, such as the older languages ALGOL, FORTRAN and PL/I, there are keywords but no reserved words, with keywords being distinguished from identifiers by other means. This makes parsing more difficult with look-ahead parsers necessary.

Distinction

The sets of reserved words and keywords in a language often coincide or are almost equal, and the distinction is subtle, so the terms are often used interchangeably. However, in careful usage they are distinguished. _... more nitty details about lexing and parsing ..._

... and we don't need to be re-debating the distinction here.


It also occurs to me that the Ruby term "keyword arguments" was poorly chosen (and again is based upon an internal implementation detail because they are gathered into a hash with the symbolized "word" as the "key" to the value.) Perhaps the term 'keyname arguments" or "named arguments" would have been better.

It is also compounded by Ruby's own exception messages ... Take this example from the ol' "Pick Axe" book, chapter 8, "More About Methods" ...

Pass in an invalid option, and Ruby complains:

def search(field, genre: nil, duration: 120)
  p [field, genre, duration ]
end

search(:title, duraton: 432)

produces: prog.rb:5:in `<main>': unknown keyword: duraton (ArgumentError)


Anyway, I've said what I feel should be done to address the confusion between reserved words, keywords and keyword arguments. Perhaps you can find some middle ground? (It doesn't bother me, as I've been programming for 40 years in many languages. And coding in Ruby for 12 years, I know the difference between these features. It is newbs that would be getting confused.)

I will not be forking or cloning this project repo just to make 1 PR.

This issue was opened for the need of a rule about parenthesizing certain method argument lists. So do what ever ya' wish if you will be the one writing the PR.

The sidebar debate over the term "keyword" could wait, or be a separate issue addressed later. (If ever.)

DanRathbun commented 4 years ago

I'll go on record that I disagree with the statement

The set of methods that have "keyword" status is not well-defined.

IMO, (as said above) ... it is really simple.

When we code, we define modules and classes. In doing so we use method calls that act as global methods because they do not need a receiver as it is implied to be self. These methods are either instance methods of Class or Module; or inherited via direct ancestry, extension or inclusion.

So these methods are identified via: (a) not needing a receiver as it is implied to be self (b) inherited from the ancestry chain: Class < Module < Object < (Kernel) < BasicObject (c) any 3rd party (gem, etc.) or Standard Library DSL methods directly included into the ancestry chain [provided they meet (a)] (d) any methods meeting (a) added to the code object being defined via extending or prepending a mixin library module

I myself will not use nor promote this rule whilst it uses the language "Always omit parenthesis ...".

The paramount rule for Ruby coding is readability. If parenthesis enhance readability they can and should be used (ie, for method calls spanning multiple lines regardless of method type.) If the method call meets the "keyword/reserved word" status it may omit parenthesis provided readability does not suffer.

An example. I always use parenthesis around arguments for block form methods calls to separate them from the beginning of the block.


I suppose it is time to fork this guide and remove what I feel are unnecessary and subjective preferential rules that just serve to take the fun out of coding, (especially the use "this" method and never it's aliases rules.)

Recently a newb was pointed toward this guide to help with coding, and the response was something like: "Uh, yea that goes on for pages that I don't wish to waste time reading because 'life is short'".

Is this a "guide" ? ... or is it a "regulation" ?

jlmuir commented 4 years ago

I'll go on record that I disagree with the statement

The set of methods that have "keyword" status is not well-defined.

If it helps at all, in the amended commit that got merged, this language is not used. Instead, it says, "it's not exactly clear which methods have "keyword" status."

I myself will not use nor promote this rule whilst it uses the language "Always omit parenthesis ...".

There are at least ten other places in the guide where it gives an "always" guideline. This PR is just using similar language.

I suppose it is time to fork this guide and remove what I feel are unnecessary and subjective preferential rules that just serve to take the fun out of coding, (especially the use "this" method and never it's aliases rules.)

Of course you're free to do that, but I'd say try opening a new issue about the wording in general first. See if you can get agreement that the wording for all or most occurrences of "always" should be changed to whatever you propose.

Recently a newb was pointed toward this guide to help with coding, and the response was something like: "Uh, yea that goes on for pages that I don't wish to waste time reading because 'life is short'".

I'm not convinced of the usefulness of that feedback. That sounds like the person doesn't want to read any guide. And that's fine, but they of course won't know what it says if they don't read it.

Is this a "guide" ? ... or is it a "regulation" ?

It's a guide, but it's not perfect.

DanRathbun commented 4 years ago

But again it is quite clear to me, even from the first day I read about these kinds of methods. Where are the reader's quotes that say it isn't clear ?

The point about the feedback is that it's too long. And IMO there are way too many rules, especially ones that go beyond "style" and formatting for readability. So I think it easier and quicker to just fork it and delete half of the extraneous stuff. (I don't have the patience to get into debates like this over every rule that isn't needed.)