From 01c71d36c635ee66db3ec4c73bbfa6afd21a2e56 Mon Sep 17 00:00:00 2001 From: Grigory Buteyko Date: Wed, 11 Dec 2024 21:29:46 +0300 Subject: [PATCH] tlgen:nolint --- Makefile | 9 +++++-- cmd/tlgen/main2.go | 33 ++++++++++-------------- internal/tlast/tllexer.go | 26 +++++++++++-------- internal/tlast/tllexer_test.go | 21 ++++++++-------- internal/tlast/tlparser_code.go | 32 ++++++++++++++++-------- internal/tlcodegen/builtin.go | 20 ++++++++------- internal/tlcodegen/tlgen.go | 40 ++++++++++++++++++++++++------ internal/tlcodegen/tlgen_kernel.go | 6 +++-- 8 files changed, 115 insertions(+), 72 deletions(-) diff --git a/Makefile b/Makefile index 3ddcc2c5..7442842f 100644 --- a/Makefile +++ b/Makefile @@ -26,13 +26,18 @@ BASIC_TL_PATH := github.com/vkcom/tl/pkg/basictl TL_BYTE_VERSIONS := ch_proxy.,ab. -.PHONY: build - all: build +.PHONY: build build: @$(GO) build -ldflags "$(COMMON_LDFLAGS)" -buildvcs=false -o target/bin/tlgen ./cmd/tlgen + +.PHONY: test +test: + @$(GO) test $(shell go list ./... | grep -v internal/tlcodegen/test/gen/) + + tlo-bootstrap: build @./target/bin/tlgen -v --language=go \ --copyrightPath=./COPYRIGHT \ diff --git a/cmd/tlgen/main2.go b/cmd/tlgen/main2.go index 750cc971..8b93dd1c 100644 --- a/cmd/tlgen/main2.go +++ b/cmd/tlgen/main2.go @@ -123,11 +123,11 @@ func runMain(opt *tlcodegen.Gen2Options) error { return fmt.Errorf("error while walkking through paths: %w", err) } for _, path := range paths { - tl, err := parseTlFile(path) + tl, err := parseTlFile(path, false, opt) if err != nil { return err } - fullTl, err := parseFullTlFile(path) + fullTl, err := parseTlFile(path, true, opt) if err != nil { return err } @@ -186,30 +186,23 @@ func runMain(opt *tlcodegen.Gen2Options) error { return nil } -func parseTlFile(file string) (tlast.TL, error) { +func parseTlFile(file string, replaceStrange bool, opt *tlcodegen.Gen2Options) (tlast.TL, error) { data, err := os.ReadFile(file) if err != nil { return nil, fmt.Errorf("error reading schema file %q - %w", file, err) } + dataStr := string(data) // Exceptions we cannot fix upstream - dataStr := strings.ReplaceAll(string(data), "_ {X:Type} result:X = ReqResult X;", "") - dataStr = strings.ReplaceAll(dataStr, "engine.query {X:Type} query:!X = engine.Query;", "") - dataStr = strings.ReplaceAll(dataStr, "engine.queryShortened query:%(VectorTotal int) = engine.Query;", "") - - tl, err := tlast.ParseTLFile(dataStr, file, false) - if err != nil { - return tl, err // Do not add excess info to already long parse error + if replaceStrange { + dataStr = strings.ReplaceAll(dataStr, "_ {X:Type} result:X = ReqResult X;", "") + dataStr = strings.ReplaceAll(dataStr, "engine.query {X:Type} query:!X = engine.Query;", "") + dataStr = strings.ReplaceAll(dataStr, "engine.queryShortened query:%(VectorTotal int) = engine.Query;", "") } - return tl, nil -} - -func parseFullTlFile(file string) (tlast.TL, error) { - data, err := os.ReadFile(file) - if err != nil { - return nil, fmt.Errorf("error reading schema file %q - %w", file, err) - } - // Exceptions we cannot fix upstream - tl, err := tlast.ParseTLFile(string(data), file, true) + tl, err := tlast.ParseTLFile(dataStr, file, tlast.LexerOptions{ + AllowBuiltin: false, + AllowDirty: false, + AllowMLC: !opt.WarningsAreErrors, + }, opt.ErrorWriter) if err != nil { return tl, err // Do not add excess info to already long parse error } diff --git a/internal/tlast/tllexer.go b/internal/tlast/tllexer.go index 92a393d5..0d5f81dc 100644 --- a/internal/tlast/tllexer.go +++ b/internal/tlast/tllexer.go @@ -129,22 +129,28 @@ func numberLexeme(s string) (string, bool) { return s[:i], allDigits } +type LexerOptions struct { + AllowBuiltin bool // allows constructor to start from '_' (underscore), used only internally by tlgen + AllowDirty bool // allows to use '_' (underscore) as constructor name, will be removed after combined.tl is cleaned up + AllowMLC bool // allow multiline comments. They are treated as warnings. +} + type lexer struct { - allowBuiltin bool - str string // iterator-like - tokens []token + opts LexerOptions + str string // iterator-like + tokens []token position Position } -func newLexer(s, file string, allowBuiltin bool) lexer { - return lexer{allowBuiltin, s, make([]token, 0, len(s)/3), Position{s, file, 1, 1, 0, 0}} +func newLexer(s, file string, opts LexerOptions) lexer { + return lexer{opts, s, make([]token, 0, len(s)/3), Position{s, file, 1, 1, 0, 0}} } // when error is returned, undefined token is added to tokens -func (l *lexer) generateTokens(allowDirty bool) ([]token, error) { +func (l *lexer) generateTokens() ([]token, error) { for l.str != "" { - err := l.nextToken(allowDirty) + err := l.nextToken() if err != nil { return l.tokens, err } @@ -195,7 +201,7 @@ func (l *lexer) checkPrimitive() bool { } } -func (l *lexer) nextToken(allowDirty bool) error { +func (l *lexer) nextToken() error { switch { case l.checkPrimitive(): return nil @@ -215,7 +221,7 @@ func (l *lexer) nextToken(allowDirty bool) error { case l.str[0] == '#': return l.lexNumberSign() case l.str[0] == '_': - if l.allowBuiltin { + if l.opts.AllowBuiltin { w := builtinIdent(l.str) if w == "_" { l.advance(len(w), ucIdent) // for TypeDecls that do not exist @@ -224,7 +230,7 @@ func (l *lexer) nextToken(allowDirty bool) error { } return nil } - if allowDirty { + if l.opts.AllowDirty { l.advance(1, lcIdent) return nil } diff --git a/internal/tlast/tllexer_test.go b/internal/tlast/tllexer_test.go index f2472bc1..ef9313e1 100644 --- a/internal/tlast/tllexer_test.go +++ b/internal/tlast/tllexer_test.go @@ -61,12 +61,12 @@ stat#9d56e6b2 %(Dictionary string) = Stat; dictionaryField {t:Type} key:string value:t = DictionaryField t; dictionary#1f4c618f {t:Type} %(Vector %(DictionaryField t)) = Dictionary t; ` - var err error - require.NoError(t, err) + t.Run("Full file", func(t *testing.T) { str := combinedBytes - lex := newLexer(str, "", false) - tokens, _ := lex.generateTokens(false) // TODO - what if err? + lex := newLexer(str, "", LexerOptions{}) + tokens, err := lex.generateTokens() + require.NoError(t, err) require.Equal(t, 0, countToken(tokens, undefined)) recombined := lex.recombineTokens() require.Equal(t, str, recombined) @@ -74,16 +74,17 @@ dictionary#1f4c618f {t:Type} %(Vector %(DictionaryField t)) = Dictionary t; t.Run("Empty file", func(t *testing.T) { str := "" - lex := newLexer(str, "", false) - _, _ = lex.generateTokens(false) + lex := newLexer(str, "", LexerOptions{}) + _, err := lex.generateTokens() + require.NoError(t, err) recombined := lex.recombineTokens() require.Equal(t, str, recombined) }) t.Run("Upper case in tag", func(t *testing.T) { str := "foo#1234567F = Foo;" - lex := newLexer(str, "", false) - _, err = lex.generateTokens(false) + lex := newLexer(str, "", LexerOptions{}) + _, err := lex.generateTokens() require.EqualError(t, err, "expect tag with exactly 8 lowercase hex digits here") }) @@ -92,8 +93,8 @@ dictionary#1f4c618f {t:Type} %(Vector %(DictionaryField t)) = Dictionary t; from := rand.Intn(len(combinedBytes)) to := from + rand.Intn(len(combinedBytes)-from) str := combinedBytes[from:to] - lex := newLexer(str, "", false) - _, _ = lex.generateTokens(false) + lex := newLexer(str, "", LexerOptions{}) + _, _ = lex.generateTokens() // returns errors, but recombination still works recombined := lex.recombineTokens() require.Equal(t, str, recombined) } diff --git a/internal/tlast/tlparser_code.go b/internal/tlast/tlparser_code.go index d2d4ea8e..3635647f 100644 --- a/internal/tlast/tlparser_code.go +++ b/internal/tlast/tlparser_code.go @@ -8,8 +8,10 @@ package tlast import ( "fmt" + "io" "log" "math" + "os" "strconv" "strings" ) @@ -56,7 +58,7 @@ func (it *tokenIterator) skipToNewline() bool { switch tok := it.front(); tok.tokenType { case comment, whiteSpace, tab: continue - case newLine: + case newLine, eof: return true default: return false @@ -613,18 +615,13 @@ func parseCombinator(commentStart tokenIterator, tokens tokenIterator, isFunctio } func ParseTL(str string) (TL, error) { - return ParseTLFile(str, "", false) -} - -func ParseTLFile(str, file string, allowDirty bool) (TL, error) { - return ParseTL2(str, file, false, allowDirty) + return ParseTLFile(str, "", LexerOptions{AllowMLC: true}, os.Stdout) } // ParseTL2 TL := TypesSection [ type ... ] FunctionSection [ function ... ] -// allowDirty - allows to use '_' (underscore) as constructor name -func ParseTL2(str, file string, allowBuiltin, allowDirty bool) (TL, error) { - lex := newLexer(str, file, allowBuiltin) - allTokens, err := lex.generateTokens(allowDirty) +func ParseTLFile(str, file string, opts LexerOptions, errorWriter io.Writer) (TL, error) { + lex := newLexer(str, file, opts) + allTokens, err := lex.generateTokens() if err != nil { return TL{}, fmt.Errorf("tokenizer error: %w", err) } @@ -635,6 +632,19 @@ func ParseTL2(str, file string, allowBuiltin, allowDirty bool) (TL, error) { log.Panicf("invariant violation in tokenizer, %s", ContactAuthorsString) } + it := tokenIterator{tokens: allTokens} + for ; it.count() != 0; it.popFront() { + tok := it.front() + if tok.tokenType == comment && strings.HasPrefix(tok.val, "/*") { + tok.val = tok.val[:2] // do not print the whole comment, but only the first line + e1 := parseErrToken(fmt.Errorf("multiline comments are not part of language"), tok, tok.pos) + if !opts.AllowMLC { + return TL{}, e1 + } + e1.PrintWarning(errorWriter, nil) + } + } + functionSection := false var res TL @@ -655,7 +665,7 @@ func ParseTL2(str, file string, allowBuiltin, allowDirty bool) (TL, error) { continue } var td Combinator - td, rest, err = parseCombinator(commentStart, rest, functionSection, allowBuiltin) + td, rest, err = parseCombinator(commentStart, rest, functionSection, opts.AllowBuiltin) if err != nil { if functionSection { return nil, fmt.Errorf("function declaration error: %w", err) diff --git a/internal/tlcodegen/builtin.go b/internal/tlcodegen/builtin.go index 0f527a20..0d359ea5 100644 --- a/internal/tlcodegen/builtin.go +++ b/internal/tlcodegen/builtin.go @@ -138,8 +138,8 @@ func (gen *Gen2) ReplaceSquareBracketsElem(tl tlast.TL) (tlast.TL, error) { tl = append(tl, res) return tWithArgs } - var replaceRepeated func(toVector bool, insideField tlast.Field) (tlast.TypeRef, error) - replaceRepeated = func(toVector bool, insideField tlast.Field) (tlast.TypeRef, error) { + var replaceRepeated func(toVector bool, insideField tlast.Field, originalCommentRight string) (tlast.TypeRef, error) + replaceRepeated = func(toVector bool, insideField tlast.Field, originalCommentRight string) (tlast.TypeRef, error) { if len(insideField.ScaleRepeat.Rep) == 0 { return tlast.TypeRef{}, insideField.ScaleRepeat.PR.BeautifulError(fmt.Errorf("repetition with no fields is not allowed")) } @@ -160,20 +160,22 @@ func (gen *Gen2) ReplaceSquareBracketsElem(tl tlast.TL) (tlast.TL, error) { // This is experimental support for transformation of [# [int]] into [__vector] insideField.ScaleRepeat.Rep[1].ScaleRepeat.ExplicitScale = true var err error - if tWithArgs, err = replaceRepeated(true, insideField.ScaleRepeat.Rep[1]); err != nil { + if tWithArgs, err = replaceRepeated(true, insideField.ScaleRepeat.Rep[1], originalCommentRight); err != nil { return tWithArgs, err } } else if len(insideField.ScaleRepeat.Rep) != 1 || insideField.ScaleRepeat.Rep[0].FieldName != "" || insideField.ScaleRepeat.Rep[0].Mask != nil { - e1 := insideField.ScaleRepeat.PR.BeautifulError(fmt.Errorf("tlgen has to invent name for type inside brackets, please give a good name to it manually")) tWithArgs = replaceRep(insideField.ScaleRepeat.Rep) + if doLint(originalCommentRight) { + e1 := insideField.ScaleRepeat.PR.BeautifulError(fmt.Errorf("tlgen has to invent name for type inside brackets, please give a good name to it manually.")) - if gen.options.WarningsAreErrors { - return tWithArgs, e1 + if gen.options.WarningsAreErrors { + return tWithArgs, e1 + } + e1.PrintWarning(gen.options.ErrorWriter, nil) } - e1.PrintWarning(gen.options.ErrorWriter, nil) } else if insideField.ScaleRepeat.Rep[0].IsRepeated { var err error - if tWithArgs, err = replaceRepeated(false, insideField.ScaleRepeat.Rep[0]); err != nil { + if tWithArgs, err = replaceRepeated(false, insideField.ScaleRepeat.Rep[0], originalCommentRight); err != nil { return tWithArgs, err } } @@ -239,7 +241,7 @@ func (gen *Gen2) ReplaceSquareBracketsElem(tl tlast.TL) (tlast.TL, error) { newField.ScaleRepeat.ExplicitScale = true } var err error - if newField.FieldType, err = replaceRepeated(toVector, newField); err != nil { + if newField.FieldType, err = replaceRepeated(toVector, newField, newField.CommentRight); err != nil { return nil, err } newField.IsRepeated = false diff --git a/internal/tlcodegen/tlgen.go b/internal/tlcodegen/tlgen.go index ebc5a74b..67bd2e86 100644 --- a/internal/tlcodegen/tlgen.go +++ b/internal/tlcodegen/tlgen.go @@ -379,6 +379,18 @@ type Gen2 struct { componentsOrder []int } +func doLint(commentRight string) bool { + if len(commentRight) < 2 { + return true + } + for _, f := range strings.Fields(commentRight[2:]) { + if f == "tlgen:nolint" { + return false + } + } + return true +} + func (gen *Gen2) InternalPrefix() string { if gen.options.SplitInternal { return "internal." @@ -504,7 +516,12 @@ func (gen *Gen2) buildMapDescriptors(tl tlast.TL) error { gen.typeDescriptors[typeName] = append(gen.typeDescriptors[typeName], typ) } } else { - if len(typ.Modifiers) == 0 { + if len(typ.TemplateArguments) != 0 { + // @read funWithArg {fields_mask: #} => True; + pr := typ.TemplateArgumentsPR + return pr.BeautifulError(fmt.Errorf("function declaration %q cannot have template arguments", conName)) + } + if len(typ.Modifiers) == 0 && doLint(typ.CommentRight) { e1 := typ.Construct.NamePR.CollapseToBegin().BeautifulError(fmt.Errorf("function constructor %q without modifier (identifier starting with '@') not recommended", typ.Construct.Name.String())) if gen.options.WarningsAreErrors { return e1 @@ -526,7 +543,7 @@ func (gen *Gen2) buildMapDescriptors(tl tlast.TL) error { } // We temporarily allow relaxed case match. To use strict match, remove strings.ToLower() calls below if EnableWarningsSimpleTypeName && strings.ToLower(cName.Name) != typePrefix && - !LegacyEnableWarningsSimpleTypeNameSkip(cName.String()) { + !LegacyEnableWarningsSimpleTypeNameSkip(cName.String()) && doLint(typ[0].CommentRight) { e1 := typ[0].Construct.NamePR.BeautifulError(fmt.Errorf("simple type constructor name should differ from type name by case only")) e2 := typ[0].TypeDecl.NamePR.BeautifulError(errSeeHere) if gen.options.WarningsAreErrors { @@ -564,7 +581,8 @@ func checkUnionElementsCompatibility(types []*tlast.Combinator, options *Gen2Opt for _, typ := range types { conName := strings.ToLower(typ.Construct.Name.Name) if EnableWarningsUnionNamespace && typ.Construct.Name.Namespace != typ.TypeDecl.Name.Namespace && - !LegacyEnableWarningsUnionNamespaceSkip(typ.Construct.Name.Namespace, typ.TypeDecl.Name.Namespace) { + !LegacyEnableWarningsUnionNamespaceSkip(typ.Construct.Name.Namespace, typ.TypeDecl.Name.Namespace) && + doLint(typ.CommentRight) { e1 := typ.Construct.NamePR.BeautifulError(fmt.Errorf("union constructor namespace %q should match type namespace %q", typ.Construct.Name.Namespace, typ.TypeDecl.Name.Namespace)) e2 := typ.TypeDecl.NamePR.BeautifulError(errSeeHere) if options.WarningsAreErrors { @@ -575,7 +593,8 @@ func checkUnionElementsCompatibility(types []*tlast.Combinator, options *Gen2Opt if EnableWarningsUnionNamePrefix && !strings.HasPrefix(conName, typePrefix) && !strings.HasSuffix(conName, typeSuffix) && - !LegacyEnableWarningsUnionNamePrefixSkip(typ.Construct.Name.Name, typePrefix, typeSuffix) { // same check as in generateType + !LegacyEnableWarningsUnionNamePrefixSkip(typ.Construct.Name.Name, typePrefix, typeSuffix) && + doLint(typ.CommentRight) { // same check as in generateType e1 := typ.Construct.NamePR.BeautifulError(fmt.Errorf("union constructor should have type name prefix or suffix %q", typePrefix)) e2 := typ.TypeDecl.NamePR.BeautifulError(errSeeHere) if options.WarningsAreErrors { @@ -585,7 +604,8 @@ func checkUnionElementsCompatibility(types []*tlast.Combinator, options *Gen2Opt continue } if EnableWarningsUnionNameExact && conName == typePrefix && - !LegacyEnableWarningsUnionNameExactSkip(typ.Construct.Name.String()) { + !LegacyEnableWarningsUnionNameExactSkip(typ.Construct.Name.String()) && + doLint(typ.CommentRight) { e1 := typ.Construct.NamePR.BeautifulError(fmt.Errorf("union constructor name should not exactly match type name %q", typePrefix)) e2 := typ.TypeDecl.PR.BeautifulError(errSeeHere) if options.WarningsAreErrors { @@ -905,7 +925,11 @@ func GenerateCode(tl tlast.TL, options Gen2Options) (*Gen2, error) { primitiveTypes[cn.tlType] = cn } - btl, err := tlast.ParseTL2(builtinBeautifulText, "", true, false) // We need references to token positions for beautification + btl, err := tlast.ParseTLFile(builtinBeautifulText, "", tlast.LexerOptions{ + AllowBuiltin: true, + AllowDirty: false, + AllowMLC: false, + }, options.ErrorWriter) // We need references to token positions for beautification, so we decided to parse as a TL file if err != nil { return nil, fmt.Errorf("failed to parse internal builtin type representation for beautification: %w", err) } @@ -1035,8 +1059,8 @@ func GenerateCode(tl tlast.TL, options Gen2Options) (*Gen2, error) { return nil, m.PR.BeautifulError(fmt.Errorf("annotations must be lower case")) } if _, ok := allAnnotations[m.Name]; !ok { - if _, ok := gen.supportedAnnotations[m.Name]; !ok { - e1 := m.PR.BeautifulError(fmt.Errorf("annotation %q not known to tl compiler", m.Name)) + if _, ok := gen.supportedAnnotations[m.Name]; !ok && doLint(typ.CommentRight) { + e1 := m.PR.BeautifulError(fmt.Errorf("annotation %q not known to tlgen", m.Name)) if gen.options.WarningsAreErrors { return nil, e1 } diff --git a/internal/tlcodegen/tlgen_kernel.go b/internal/tlcodegen/tlgen_kernel.go index 476dbf5e..85218261 100644 --- a/internal/tlcodegen/tlgen_kernel.go +++ b/internal/tlcodegen/tlgen_kernel.go @@ -522,7 +522,8 @@ func (gen *Gen2) generateTypeStruct(lrc LocalResolveContext, myWrapper *TypeRWWr if newField.t.IsTrueType() && !newField.Bare() && newField.t.origTL[0].TypeDecl.Name.String() == "True" && newField.t.origTL[0].Construct.Name.String() == "true" && - !LegacyAllowTrueBoxed(myWrapper.origTL[0].Construct.Name.String(), field.FieldName) { + !LegacyAllowTrueBoxed(myWrapper.origTL[0].Construct.Name.String(), field.FieldName) && + doLint(field.CommentRight) { // We compare type by name, because there is examples of other true types which are to be extended // to unions or have added fields in the future e1 := field.FieldType.PR.BeautifulError(fmt.Errorf("true type fields should be bare, use 'true' or '%%True' instead")) @@ -534,7 +535,8 @@ func (gen *Gen2) generateTypeStruct(lrc LocalResolveContext, myWrapper *TypeRWWr if _, ok := newField.t.trw.(*TypeRWBool); ok { if newField.t.origTL[0].TypeDecl.Name.String() == "Bool" && newField.fieldMask != nil && !newField.fieldMask.isArith && newField.fieldMask.isField && - !LegacyAllowBoolFieldsmask(myWrapper.origTL[0].Construct.Name.String(), field.FieldName) { + !LegacyAllowBoolFieldsmask(myWrapper.origTL[0].Construct.Name.String(), field.FieldName) && + doLint(field.CommentRight) { // We compare type by name to make warning more narrow at first. e1 := field.FieldType.PR.BeautifulError(fmt.Errorf("using Bool type under fields mask produces 3rd state, you probably want to use 'true' instead of 'Bool'")) if gen.options.WarningsAreErrors {