I’ve generated scaffold named “action” once, and it seemed working at first, but had to dig rails source code and rename it later. It would be nice to have a “don’t do that” warning built into generators for all reserved names.
14 Likes
I agree! A gentle command line warning of ‘This #{insert-whatever-naming-conflicting-object} overrides a default Rails behavior from #{name-the-conflicting-space}’ you’d be covered! Intentionally overriding a method would be no big deal, it may produce a warning (and let’s say it only produces the warning in Development and not Prod by default).
It would also be nice if they gave an all out full-stop error if someone overrides one of the Private API methods so that they don’t end up accidentally building upon private methods that could be deprecated without warning.
1 Like
I like the idea of a warning. I worry about a full-stop error breaking a lot of people’s monkeypatches, though.
Well, the full-stop erring out doesn’t and probably shouldn’t come right away for that exact reason. If we graduated into it over a major version, and just issued warnings at first, then that would make the transition easier and give devs a chance to fix the bad behavior.
Yeah, warning is a good starting point.
Honestly, I also find the idea of a full-stop error pretty anti-Rubyish, especially if we’re also talking about private API accesses. Ruby is a language you can hammer into any shape you want to. I don’t think it’s possible to take away the dangers of e.g. monkeypatching private APIs without taking away the freedom to improve upon the framework that that also gives you. I see that freedom as having been important to Rails’s applicability as a framework, so I’m loath to give it up.
Since i’ve generated it from command line, and most people are probably doing it the same way, a bright yellow/red “watch yourself” warning could be enough. The same actually goes for attributes - you should be able to overwrite methods you don’t use, but then again - you should know about it in advance, renaming database columns can be painful. Warning will do the trick.
2 Likes
Long ago, when the documentation for Rails was maintained in a Wiki, there was a section devoted to reserved words. When that wiki collapsed, over ten years ago, I captured the list and used it to seed this site: https://reservedwords.herokuapp.com/. It hasn’t had a lot of love lately, and it still runs on Rails 2.3, but it’s here if you want to contribute, add comments if anything has stopped being a problem (that’s what I’d expect), and maybe you’ll find something surprising here.
Walter
5 Likes
Nice one, i guess i could sort it out and convert to YML if this feature gets accepted. Although i don’t think it should include ruby core/stdlib reserved words like URI or ARGV, only stuff like attribute, type, action etc.
Thanks! I had a bit of a panic last night after I posted this, I discovered that the Discus comments weren’t loading because the reference to their JavaScript was http:// instead of https://. Oh, what fun I had trying to post a ten-year-old app to Heroku! It took me at least 5 tries, with increasingly elderly Ruby versions and Bundler versions (thank heaven I had added Bundler at some point in the past). If this app becomes popular again, I may have to upgrade it to something more recent. I have until November to move it off of the Cedar stack, in any case…
Walter
Well as it turns out, I was binging some old Rails conference videos on YouTube while doing some finish work on a new standing desk, and I heard Eileen Uchitelle (eileencodes on Github, one of the maintainers of Rails) that they really don’t want people using the private Rails API methods at all, and that they don’t feel sorry at all if you’ve been using private API methods and they just remove/deprecate them without warning.
So yeah, sounds like a warning is good enough and it’ll just suck to be the person who monkey patches everything and ignores the warnings 
I’ll second the request for this to include attribute name checking. I was just doing a code review for a relatively new to rails coder and asked him to rename the attribute he called “type” because “I’m not sure if you will run into reserve word issues here… but this very word has gotten me in trouble in the past.”
1 Like
There is one more thing, and probably an easier way towards stable implementation - most of the wrong ways of naming models/attributes actually fail default tests. However, once you have scaffolded something, and got no errors, you expect it to work, and won’t run test.
While it is not a good idea to force tests after generators (they may fail for other reasons), it is possible to take a list of all methods and classes rails uses, run tests for them as model/attribute names, and then compose a list of failing ones.
Active Record already warns you if your table contains a column that clashes with it’s internal methods: rails/attribute_methods.rb at e61bdbf315851ac1cdcd2f8ac6ebac8fe7c7261b · rails/rails · GitHub
Here is a more detailed example:
rails new smthcd smthrails g scaffold action type:stringrails db:migrate- No errors or warnings, everything seems fine
However, if i run server and try to create a new ‘action’ record, or just run tests, there is an error:
NoMethodError (undefined method `permit' for "create":String)
Which leaves you digging rails source code in a state of great WTF.
This comes from controllers, which either don’t check names, or do it wrong.
If i rename model to something valid, but leave type as a column name, i can get somewhat helpful error
This error is raised because the column 'type' is reserved
However, this error is reproduceable only at certain conditions, and somehow ignored by default tests.
type isn’t exactly a reserved name, and can’t be banned.
It is the default name for STI columns.
Try generating a column named save, or update and Rails will warn you, but type is a valid column, it simply have an implicit use.
I get how it’s easy to fall in this, and maybe STI should require an explicit declaration of the STI column, but it doesn’t mean AR doesn’t warn about conflicting columns.
The point here is that some columns/model names are unobviously invalid, and it would be nice if rails would warn about them even by using manually composed blacklist. Type is not a valid column in default configuration, and for most developers it is better to use another name, rather than reconfiguring AR.
But what if I actually meant to have a type:String column to be used for single table inheritance? Wouldn’t that be just as confusing to see a warning that tells me I’m likely making a mistake?
Not if it would say, that type is intended for single table inheritance, and if you were going to use it for something else, then rename. STI is not as common as word ‘type’ for something else.