Avoid class A::B definitions

[teoljungberg]

ctags is an extremely useful tool for navigating quickly to a class,
module, or method definition. Sadly it has a few limitations, one of
them is how namespaced classes/modules are defined.

If we define a class B like so: class A::B; end, B will never be
found nor indexed by ctags.

By defining classes like:

module A
  class B
  # body omitted
  end
end

We can use ctags to navigate to B (and A for that matter aswell`)

[derekprior]

I'm torn on this. Does ctags pick up nested classes like so?

class A
  class B
  # body omitted
  end
end

or does it always have to be a class within a module? There are times when Ruby won't let you do this without nesting (or re-opening) a class. For instance, if you have a top level Comment class and want a class in that namespace you have to nest it inside a class definition. You can't declare a class and then declare that class as a module elsewhere.

Are there any other advantages at all to explicitly declaring the module like that? I love ctags but I'm not sure I'm willing to make structural concessions to support it.

[teoljungberg]

I'm torn on this. Does ctags pick up nested classes like so?

All nested classes and modules will be picked up.

I don't really follow your example @derekprior, could you clarify?

[calleluks]

How we define classes and modules also affect constant lookup:

module A
  CONSTANT = "a constant"
end

module A::B
  def self.test
    CONSTANT
  end
end

module A
  module C
    def self.test
      CONSTANT
    end
  end
end

A::B.test #=> NameError: uninitialized constant A::B::CONSTANT
A::C.test #=> "a constant"

[teoljungberg]

good example @calleerlandsson, that one has bit me once or twice

[derekprior]

All nested classes and modules will be picked up.

Okay, thats cool. I was just saying that sometimes it's necessary to nest a class inside of another class (rather than a module) to get the name you want (i.e. your top level is already a class rather than a module). I mistook your PR description for saying I could only nest into modules.

@calleerlandsson interesting, thanks.

👍

[JoelQ]

This seems reasonable. I knew this was a general best practice due to the constant lookup issues mentioned by @calleerlandsson but wasn't aware this also made things easier for CTags.

👍

[calleluks]

Would the guideline be clearer if it was more than an example? Maybe something like "Avoid defining classes and modules using shorthand nesting (class A::B)"?

[teoljungberg]

@calleerlandsson Agreed, updated in 5779588

[gylaz]

Should we flip this wording to match the example?
Something like: Prefer nested module definitions instead of the shorthand version

[teoljungberg]

I sort-off want to keep Avoid in there to emphesize that we couldn't use A::B unless it's actually needed

[gylaz]

👍

[drapergeek]

👍

[gabebw]

Sure. I've been split 50/50 in the past, basically however I feel that day. The A::B.test example is convincing. 👍

[mcmire]

All the times I've nested modules (and granted I haven't done it that much) have been for convenience. There's no real need to do this though. This seems like a good guideline to me.

[teoljungberg]

This has been open for more than a week now, and there doesn't seem to be a strong argument against this. Does anyone else have any objections before I squash and merge?

[calleluks]

Should "instead of" be "over" or "to"?

[teoljungberg]

Good call, fixed in 7b95965

[TheLonelyGhost]

👍 for merging the guideline. I usually nest the modules instead of the single-line definition, at least until load order begins to matter. Then it can be used as an early warning system with an obvious solution.

# file a
module A
  class B
    # ...
  end
end


# file 2
module A
  class C
    # ...
  end
end


# file 3
class A::D
  # ...
end

In the above code, load order only matters with file 3 since it requires module A to be defined before it can define class A::D. In most circumstances, load order does not matter so long as all appropriate files are actually loaded.

Using syntax in file 3 adds an additional requirement that it be loaded after file 1 or 2, which shouldn't be necessary.

[teoljungberg]

No objections made, merging