laravel-ide-helper icon indicating copy to clipboard operation
laravel-ide-helper copied to clipboard

Allowing table inspection to gracefully fail so other IDE helpers for models may still be generated

Open clayzar opened this issue 6 months ago • 3 comments

Summary

This adds logic to allow table inspection to gracefully fail. More specifically, this allows other the doc block generation to continue and generate other helpers that aren't database related.

A use case for this is for NoSQL database drivers, for which schema inspection doesn't make sense or doesn't have a convention to determine columns, yet is still helpful to have a handful of the other IDE helpers that this package is capable of generating. Currently, when using the latest Laravel MongoDB package the corresponding driver does not implement the compileColumns method. So we see an error thrown that mentions this undefined method when running the command to generate doc blocks for models configured to use a Mongo DB connection.

I've linked a couple of issues that mention this.

I've also added silence-able warnings if table properties could not be inspected for a given model. A warning is output as the command generates output and another at the end of the output denoting the models whose table could not be inspected, to ensure the developer is aware to scroll up should the output be many lines long.

https://github.com/barryvdh/laravel-ide-helper/issues/590 https://github.com/mongodb/laravel-mongodb/issues/1785

Type of change

This could be thought of as a bug fix, new feature, or misc change – depending on how you look at it.

  • [x] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [x] This change requires a documentation update
  • [x] Misc. change (internal, infrastructure, maintenance, etc.)

Checklist

  • [ ] Existing tests have been adapted and/or new tests have been added
  • [ ] Add a CHANGELOG.md entry
  • [ ] Update the README.md
  • [ ] Code style has been fixed via composer fix-style

clayzar avatar Feb 22 '24 23:02 clayzar

Great thoughts here! I've made those adjustments and also tucked away the updates more neatly. Just to regurgitate what you had in mind:

If the ide-helpers.silenced_models value is null or otherwise missing (such would be the case for current projects using the package).. no behavior changes, the exception will be thrown if it fails.

Otherwise, this will gracefully handle an exception when inspecting the table and provide warnings in the output. Those warnings can be silenced by specifying the FQCN of the class in ide-helper.silenced_models.

I've added the exception class and message to the output. Here's a sample of the output in an example project with 2 models that use a MongoDB connection

Screenshot 2024-02-23 at 4 52 32 PM

clayzar avatar Feb 23 '24 23:02 clayzar

I feel that setting the value of silenced_models to an empty array to allow graceful error handling isn't very intuitive.

With the warning collection, I was trying to map out something that could be used in case other warnings were ever possible, but I realize this may be a little overblown for the need here.

Thinking back on the primary goal here, I think it may be simpler to just use a config setting named skip_model_tables that defaults to an empty array. This would default to the same behavior as today (disabled) and must be enabled deliberately. The items in the array would be FQCN for any model for which table inspection should be skipped. The value could also be set to * to skip all models without having to explicitly list each one, helpful for large code-bases exclusively using an unsupported database driver.

Thoughts?

clayzar avatar Feb 27 '24 19:02 clayzar

Thoughts?

Let's see code and I suggest you make (another) PR to discuss this!

mfn avatar Mar 04 '24 20:03 mfn