apigear-io / cli

The ApiGear client as go project
https://apigear.io
MIT License
1 stars 2 forks source link

add new visitor to solution and targets to better support DB linting #109

Closed jryannel closed 10 months ago

jryannel commented 10 months ago

This adds a new visitor to the spec documents to allow not only lint for FS based documents but also for DB based documents.

jryannel commented 10 months ago

One thing I don't like from the solution that it makes things more complicated. Much more complicated. E.g. by adding a visitor, a lint visitor, each struct needs to have a accept function. It is more generic, yes.

The solution comes from the problem that when having a doc in the hub the inputs will be document ids and not file paths. So we can not do the normal expansion or validation. We first have to transform the doc ids to real paths on the FS.

The documents are loaded and created from JSON so we can not use a particular construct either. Or can we?

The code normally is:

var doc SolutionDoc
json.Unmarshall(bytes, doc)

So we could control the creation by passing in a linter (e.g. an interface which has a Lint function) which would be called by a lint function.

We could then make the type private and only expose a MakeSolutionDoc function.


type SolutionDocLinter {
  Lint(doc * SolutionDoc) error
}

func (s *SolutionDoc) Lint() {
  s.linter.Lint(s)
}

This seems to be much more straight forward.

Do we need this anyway?

We need to have a way to lint documents on the hub before they are written to the FS. So we will always need the FS part.

We could also in general just replace the doc IDs with the FS path and then call lint. Then there would be no need for a DB linting. The DB linting would then be a function inside the hub which sees the doc as external doc.

Also if we would have the DB linting as interface we would need to pass first the DB linting and then the FS linting, e.g. a chained linting. But the DB linting is more a transformation.

jryannel commented 10 months ago

I like the idea that the lint code is split between onStart, onFix and onFinish. It gives a better split between the code. I don't like the visitor pattern. I guess I will change the impl to have a Lint call on the doc which does by default (e.g. lint is nil a FS linting). This should be fairly easy. And the code will be in a sol_doc_lint.go code but be part of the sol_doc (so the SolutionDoc is a linter, the default linter).

This means we only have to add a lint interface and a lint method and three methods to the solution doc. This is much more lightweight than the visitor pattern.

jryannel commented 10 months ago

Does not make sense.