Baffling error when migration includes upper-case

Right now, if you accidentally give a migration filename an upper-case character (say, 001-initialMigration.rb), you get a crash:

You have a nil object when you didn’t expect it! You might have expected an instance of Array.

The error occurred while evaluating nil.first /home/bronson/workspace-hobo/hobo-docs/vendor/rails/activerecord/lib/active_record/migration.rb:367:in `migration_files’

I would like to fix this.

Isn’t this an artificial limitation? Why not just add “A-Z” to the regex in migration.rb so that migration names can contain capitals?

  def migration_version_and_name(migration_file)
  •    return *migration_file.scan(/([0-9]+)_([_a-z0-9]*).rb/).first
    
  •    return *migration_file.scan(/([0-9]+)_([_A-Za-z0-9]*).rb/).first
    end
    

Either way, should I submit a patch that does something like this?

  • migration_version_and_name(f).first.to_i
    
  • m = migration_version_and_name(f)
    
  • raise IllegalMigrationNameError.new(f) if m.nil?
    
  • m.first.to_i
    

(and, of course, create the IllegalMigrationNameError object)

That way, at least, Rails will spit out an intelligible error message instead of bombing out when it calls nil.first.

Does this make sense?

- Scott

Right now, if you accidentally give a migration filename an upper-case character (say, 001-initialMigration.rb), you get a crash:

   You have a nil object when you didn't expect it!    You might have expected an instance of Array.    The error occurred while evaluating nil.first

/home/bronson/workspace-hobo/hobo-docs/vendor/rails/activerecord/lib/active_record/migration.rb:367:in `migration_files'

I would like to fix this.

Isn't this an artificial limitation? Why not just add "A-Z" to the regex in migration.rb so that migration names can contain capitals?

      def migration_version_and_name(migration_file) - return *migration_file.scan(/([0-9]+)_([_a-z0-9]*).rb/).first + return *migration_file.scan(/([0-9]+)_([_A-Za-z0-9]*).rb/).first       end

I wrote about this some time ago:

http://talklikeaduck.denhaven2.com/articles/2007/09/11/conventions-uber-alles

prompted by this, Ben Scofield proposed a similar patch: http://dev.rubyonrails.org/ticket/9542

Which was rejected because it would violate roundtrips from classname to filename.

Either way, should I submit a patch that does something like this?

- migration_version_and_name(f).first.to_i + m = migration_version_and_name(f) + raise IllegalMigrationNameError.new(f) if m.nil? + m.first.to_i

(and, of course, create the IllegalMigrationNameError object)

That way, at least, Rails will spit out an intelligible error message instead of bombing out when it calls nil.first.

Well why not submit it and see what the maintainers have to say.

Right now, if you accidentally give a migration filename an upper-case character (say, 001-initialMigration.rb), you get a crash:

Was this file made from the migration generator or by hand? It seems like we should fix this if it was somehow generated improperly but hand-written migrations should probably need to follow convention.

Either way, should I submit a patch that does something like this?

  • migration_version_and_name(f)
    

.first.to_i

  • m = migration_version_and_name(f)
    
  • raise IllegalMigrationNameError.new(f) if m.nil?
    
  • m.first.to_i
    

(and, of course, create the IllegalMigrationNameError object)

That way, at least, Rails will spit out an intelligible error message instead of bombing out when it calls nil.first.

That’s a great idea. Well-named errors (especially on the command line) are a real help to new Railers (and old ones too).

::Jack Danger

prompted by this, Ben Scofield proposed a similar patch: http://dev.rubyonrails.org/ticket/9542 Which was rejected because it would violate roundtrips from classname to filename.

That makes total sense. Given that, I agree that migrations filenames should be restricted to lower case.

Was this file made from the migration generator or by hand? It seems like we should fix this if it was somehow generated improperly but hand-written migrations should probably need to follow convention.

It was made using the Hobo migration generator which apparently doesn’t sanitize the filenames. I’ll open a Hobo ticket.

Either way, should I submit a patch…? That’s a great idea. Well-named errors (especially on the command line) are a real help to new Railers (and old ones too).

Ticket: http://dev.rubyonrails.org/ticket/9909

Please vote for it! :slight_smile:

- Scott