Throw errors when dots are in fields in targets #1699
Conversation
| @@ -89,7 +89,7 @@ export class Assertion extends ActionBuilder<dataform.Assertion> { | |||
| } | |||
|
|
|||
| if (!config.name) { | |||
| config.name = Path.fileName(config.filename); | |||
| config.name = Path.basename(config.filename); | |||
There was a problem hiding this comment.
i still think this capitalized import name is super weird (Path as opposed to path which would be more standard)
There was a problem hiding this comment.
It should probably be called pathUtils instead. It's capitalized for importing our path.ts file, but lowercase for our node path imports
Line 5 in b5368e0
Will update in a subsequent CL.
There was a problem hiding this comment.
IMO it's fine as path everywhere, but up to you
core/session.ts
Outdated
| @@ -392,6 +392,16 @@ export class Session { | |||
| ) | |||
| ); | |||
|
|
|||
| this.checkNoDotsInTargetFields( | |||
There was a problem hiding this comment.
why do we need to do this at the end of compilation? IMO it should be happening well before then - at action declaration time.
There was a problem hiding this comment.
Where are you suggesting to put it?
Would it be both:
- Action functions in the session https://github.com/dataform-co/dataform/blob/b5368e04a047c6f2c958b6dd14d9bd5f872e4e0f/core/session.ts#L236C11-L236C21
- Action constructors
dataform/core/actions/table.ts
Line 291 in b5368e0
There was a problem hiding this comment.
Everything passes through the action constructors, right? So we should only do it there.
…e-errors Throw errors when dots are in fields in targets
Fixes #1362 (comment), which contains the explanation for this solution
Also update filename in utils to basename as it is more correct.