fix(entity-generator): allow arbitrary class and prop names as identifiers

[boenrobot]

AbstractNamingStrategy is updated to handle class conflicts, and inherently,
any naming strategy that inherits from it.

The "word boundary" character in entity and property names is now correctly replaced,
even in cases where multiple sequential separators are present.

Any table that starts with a character that is invalid as a JS start character (not just digits)
gets prefixed with "E".
Characters that are not valid in any position as a class identifier (or the character "$")
get turned into code points, prefixed with $, making them valid (though inconvenient to type)
identifiers and file names.

In AbstractNamingStrategy.columnNameToProperty(),
prop names that would be in conflict with PopulatePath values get prefixed with "$".

Prop names that are not valid identifiers get string quoted.
This is a SourceFile/EntitySchemaSourceFile feature, not a naming strategy feature,
so it will get applied regardless of naming strategy.

The MySqlSchemaHelper doesn't special case index expression based on column name.
Lack of column name or presence of an "expression" column with "where" is still considered.

The PostgreSqlSchemaHelper properly escapes all identifiers when looking them up.
Due to the relative complexity of PostgreSQL's expressions vs column names,
index properties are still not picked up for odd identifiers,
though an index expression is generated.

Also made the imports of types tied to the "entity" prop option, rather than the prop declaration.
This ensures proper imports when using mapToPk in extension hooks.

Also moved the schema de-duplication code to happen before bi-directional and many-to-many
detection, to ensure less work and error prone-ness for the other stages in updating references.

Also fixed quoting of strings in generated columns.

[boenrobot]

This started as yet another subset of #5273 (and I know it will have conflicts with it, which I'll resolve later...), but it evolved as I realized there's no test for the thing I noticed there (the inability to correctly reference "odd" identifiers like that, due to the amendment being outside of the naming strategy), and during the making of the test, I realized there's also plenty of other cases where the naming strategy is insufficient, and eventually reached this.

There is one known edge case not covered here... Identifiers containing a dot (.). This would require a bit more significant refactoring to make MirkoORM handle this better, so... a story for a different PR to specifically handle "dot in identifier vs dot to separate schema and table name" in some way... I think it's best to introduce dedicated schema fields in the FK, and output those too in separate prop options where not the current schema... but again, that is a more significant refactor.

NOTE: The change in the schema generator is a consequence of the quote normalization happening in loadInformationSchema.
It's probably worth to also normalize the charset of strings there too, or in higher layers, though I would still need to test what MySQL defaults to... I suspect charset of the connection that defined the column, which means that to "properly" detect schema changes, the schema generator needs to lookup its connection's charset, and only strip that one out from the generated column definitions it is comparing.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.77%. Comparing base (acd6590) to head (e7c91b4).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #5359   +/-   ##
=======================================
  Coverage   99.77%   99.77%           
=======================================
  Files         260      260           
  Lines       17868    17886   +18     
  Branches     3780     3793   +13     
=======================================
+ Hits        17827    17845   +18     
  Misses         41       41           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[B4nan]

Prop names that conflict with the prototype of Reference (which all loaded entities inherit from)

This is not true. Why did you think so? Maybe we need to improve the docs somewhere.

Reference is an optional wrapper class, nowadays represented by the Ref type.

https://mikro-orm.io/docs/guide/type-safety#reference-wrapper

[boenrobot]

Prop names that conflict with the prototype of Reference (which all loaded entities inherit from)

This is not true. Why did you think so? Maybe we need to improve the docs somewhere.

Reference is an optional wrapper class, nowadays represented by the Ref type.

https://mikro-orm.io/docs/guide/type-safety#reference-wrapper

Ok, "inherit" was the wrong word... "wrap"... point is that you have a point at which you have accessible both the properties and extra identifiers like "$", and "load".

So... when you have a wrapped object, be it because of Ref, or because it is the direct result of find()... Is there a way to read or write a prop with an arbitrary name? If there is, it is not documented.

[B4nan]

point is that you have a point at which you have accessible both the properties and extra identifiers like "$", and "load".

Nope, that's not how it works, the wrapped entity doesn't have anything in common with the Reference class.

So... when you have a wrapped object, be it because of Ref, or because it is the direct result of find()... Is there a way to read or write a prop with an arbitrary name? If there is, it is not documented.

I don't understand your question. It's a simple wrapper class, if you want to access anything else than the primary key, you need to unwrap with one of the helper methods.

[B4nan]

the escaping is apparently not working correctly

[B4nan]

wrong escaping

[boenrobot]

If you mean the double slash... this is an artifact of the fact that jest snapshots aren't the literal file, but a template string inside a js file.

If you check the actual file produced, you'll see it has only one slash inside it.

That said, I should probably make it create the files, as in the mysql test, so... I'll change that part. But the escaping here is correct.

EDIT: OK, changed to output the file, and check for a file presence and remove... same as MySQL test.

[B4nan]

unnecessary escaping of `

[boenrobot]

I think This is the same "problem" here too...

The snapshot is inside a template string... that's just how jest's .snap files work...

So it escapes the `, in order to stay into the template string, while in the actual contents of the file, you won't see the ` being escaped.

[boenrobot]

I've added a fix for the first problem described in #5400, meaning a custom naming strategy would not be needed for that part. Also amended the tests to feature a new table and a column that follow those patterns.

And like I already said about the backslash and backtick escapes... That this is just because the snapshots are template strings. The raw files don't have the extra slashes. So those are correct.

[boenrobot]

As mentioned in #5418, I've now added disambiguation for column names that could end up conflicting with "$infer" or "*", which yes, are technically valid identifiers, even if no one in their right mind would use those actual names for an actual column.

[B4nan]

I hope this wont backfire, the changes are quite a magic to me :]