From 3f348ae222122b9d908c39abc8bd2c28714205b3 Mon Sep 17 00:00:00 2001 From: Peter Edge Date: Sat, 15 Sep 2018 00:18:52 +0200 Subject: [PATCH] Add --config-data flag (#224) --- CHANGELOG.md | 1 + internal/cmd/cmd_test.go | 47 ++++++++++++++++- internal/cmd/flags.go | 5 ++ internal/cmd/templates.go | 34 ++++++++++++ internal/exec/exec.go | 7 +++ internal/exec/runner.go | 15 +++++- internal/file/file.go | 9 ++++ internal/file/proto_set_provider.go | 79 +++++++++++++++++++--------- internal/settings/config_provider.go | 24 +++++++++ internal/settings/settings.go | 8 ++- 10 files changed, 200 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b83f74b..874eb5ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added - Accept `prototool.json` files for configuation in addition to `prototool.yaml` files. +- Add `--config-data` flag. ## [1.2.0] - 2018-08-29 diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go index b9a96100..9da0714e 100644 --- a/internal/cmd/cmd_test.go +++ b/internal/cmd/cmd_test.go @@ -354,6 +354,49 @@ func TestLint(t *testing.T) { ) } +func TestLintConfigDataOverride(t *testing.T) { + cwd, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir("testdata/lint/gopackagelongform")) + defer func() { + require.NoError(t, os.Chdir(cwd)) + }() + assertDoLintFile( + t, + false, + `5:1:FILE_OPTIONS_GO_PACKAGE_NOT_LONG_FORM`, + "gopackagelongform.proto", + "--config-data", + `{"lint":{"rules":{"remove":["FILE_OPTIONS_EQUAL_GO_PACKAGE_PB_SUFFIX"]}}}`, + ) + assertDoLintFile( + t, + false, + `5:1:FILE_OPTIONS_EQUAL_GO_PACKAGE_PB_SUFFIX`, + "gopackagelongform.proto", + "--config-data", + `{"lint":{"rules":{"remove":["FILE_OPTIONS_GO_PACKAGE_NOT_LONG_FORM"]}}}`, + ) + assertDoLintFile( + t, + false, + `5:1:FILE_OPTIONS_EQUAL_GO_PACKAGE_PB_SUFFIX + 5:1:FILE_OPTIONS_GO_PACKAGE_NOT_LONG_FORM`, + "gopackagelongform.proto", + "--config-data", + `{}`, + ) + assertExact( + t, + 1, + `json: unknown field "unknown_key"`, + "lint", + "gopackagelongform.proto", + "--config-data", + `{"unknown_key":"foo"}`, + ) +} + func TestGoldenFormat(t *testing.T) { t.Parallel() assertGoldenFormat(t, false, false, "testdata/format/proto3/foo/bar/bar.proto") @@ -709,7 +752,7 @@ func assertDoCreateFile(t *testing.T, expectSuccess bool, remove bool, filePath } } -func assertDoLintFile(t *testing.T, expectSuccess bool, expectedLinePrefixesWithoutFile string, filePath string) { +func assertDoLintFile(t *testing.T, expectSuccess bool, expectedLinePrefixesWithoutFile string, filePath string, args ...string) { lines := getCleanLines(expectedLinePrefixesWithoutFile) for i, line := range lines { lines[i] = filePath + ":" + line @@ -718,7 +761,7 @@ func assertDoLintFile(t *testing.T, expectSuccess bool, expectedLinePrefixesWith if !expectSuccess { expectedExitCode = 255 } - assertDo(t, expectedExitCode, strings.Join(lines, "\n"), "lint", filePath) + assertDo(t, expectedExitCode, strings.Join(lines, "\n"), append([]string{"lint", filePath}, args...)...) } func assertDoLintFiles(t *testing.T, expectSuccess bool, expectedLinePrefixes string, filePaths ...string) { diff --git a/internal/cmd/flags.go b/internal/cmd/flags.go index 7d699c15..005cee32 100644 --- a/internal/cmd/flags.go +++ b/internal/cmd/flags.go @@ -28,6 +28,7 @@ type flags struct { address string cachePath string callTimeout string + configData string connectTimeout string data string debug bool @@ -63,6 +64,10 @@ func (f *flags) bindCallTimeout(flagSet *pflag.FlagSet) { flagSet.StringVar(&f.callTimeout, "call-timeout", "60s", "The maximum time to for all calls to be completed.") } +func (f *flags) bindConfigData(flagSet *pflag.FlagSet) { + flagSet.StringVar(&f.configData, "config-data", "", "The configuration data to use instead of reading prototool.yaml or prototool.json files.\nThis will act as if there is a configuration file with the given data in the current directory, and no other configuration files recursively.\nThis is an advanced feature and is not recommended to be generally used.") +} + func (f *flags) bindConnectTimeout(flagSet *pflag.FlagSet) { flagSet.StringVar(&f.connectTimeout, "connect-timeout", "10s", "The maximum time to wait for the connection to be established.") } diff --git a/internal/cmd/templates.go b/internal/cmd/templates.go index 9db3f824..1a8547f5 100644 --- a/internal/cmd/templates.go +++ b/internal/cmd/templates.go @@ -45,6 +45,7 @@ var ( return runner.All(args, flags.disableFormat, flags.disableLint, flags.fix) }, BindFlags: func(flagSet *pflag.FlagSet, flags *flags) { + flags.bindConfigData(flagSet) flags.bindDisableFormat(flagSet) flags.bindDisableLint(flagSet) flags.bindJSON(flagSet) @@ -60,6 +61,9 @@ var ( Run: func(runner exec.Runner, args []string, flags *flags) error { return runner.BinaryToJSON(args) }, + BindFlags: func(flagSet *pflag.FlagSet, flags *flags) { + flags.bindConfigData(flagSet) + }, } cleanCmdTemplate = &cmdTemplate{ @@ -80,6 +84,7 @@ var ( return runner.Compile(args, flags.dryRun) }, BindFlags: func(flagSet *pflag.FlagSet, flags *flags) { + flags.bindConfigData(flagSet) flags.bindDryRun(flagSet) flags.bindJSON(flagSet) flags.bindProtocURL(flagSet) @@ -152,6 +157,7 @@ If Vim integration is set up, files will be generated when you open a new Protob return runner.Create(args, flags.pkg) }, BindFlags: func(flagSet *pflag.FlagSet, flags *flags) { + flags.bindConfigData(flagSet) flags.bindPackage(flagSet) }, } @@ -163,6 +169,9 @@ If Vim integration is set up, files will be generated when you open a new Protob Run: func(runner exec.Runner, args []string, flags *flags) error { return runner.DescriptorProto(args) }, + BindFlags: func(flagSet *pflag.FlagSet, flags *flags) { + flags.bindConfigData(flagSet) + }, } downloadCmdTemplate = &cmdTemplate{ @@ -172,6 +181,9 @@ If Vim integration is set up, files will be generated when you open a new Protob Run: func(runner exec.Runner, args []string, flags *flags) error { return runner.Download() }, + BindFlags: func(flagSet *pflag.FlagSet, flags *flags) { + flags.bindConfigData(flagSet) + }, } fieldDescriptorProtoCmdTemplate = &cmdTemplate{ @@ -181,6 +193,9 @@ If Vim integration is set up, files will be generated when you open a new Protob Run: func(runner exec.Runner, args []string, flags *flags) error { return runner.FieldDescriptorProto(args) }, + BindFlags: func(flagSet *pflag.FlagSet, flags *flags) { + flags.bindConfigData(flagSet) + }, } filesCmdTemplate = &cmdTemplate{ @@ -190,6 +205,9 @@ If Vim integration is set up, files will be generated when you open a new Protob Run: func(runner exec.Runner, args []string, flags *flags) error { return runner.Files(args) }, + BindFlags: func(flagSet *pflag.FlagSet, flags *flags) { + flags.bindConfigData(flagSet) + }, } formatCmdTemplate = &cmdTemplate{ @@ -200,6 +218,7 @@ If Vim integration is set up, files will be generated when you open a new Protob return runner.Format(args, flags.overwrite, flags.diffMode, flags.lintMode, flags.fix) }, BindFlags: func(flagSet *pflag.FlagSet, flags *flags) { + flags.bindConfigData(flagSet) flags.bindDiffMode(flagSet) flags.bindJSON(flagSet) flags.bindLintMode(flagSet) @@ -217,6 +236,7 @@ If Vim integration is set up, files will be generated when you open a new Protob return runner.Gen(args, flags.dryRun) }, BindFlags: func(flagSet *pflag.FlagSet, flags *flags) { + flags.bindConfigData(flagSet) flags.bindDryRun(flagSet) flags.bindJSON(flagSet) flags.bindProtocURL(flagSet) @@ -299,6 +319,7 @@ $ cat input.json | prototool grpc example \ return runner.GRPC(args, flags.headers, flags.address, flags.method, flags.data, flags.callTimeout, flags.connectTimeout, flags.keepaliveTime, flags.stdin) }, BindFlags: func(flagSet *pflag.FlagSet, flags *flags) { + flags.bindConfigData(flagSet) flags.bindAddress(flagSet) flags.bindCallTimeout(flagSet) flags.bindConnectTimeout(flagSet) @@ -331,6 +352,9 @@ $ cat input.json | prototool grpc example \ Run: func(runner exec.Runner, args []string, flags *flags) error { return runner.JSONToBinary(args) }, + BindFlags: func(flagSet *pflag.FlagSet, flags *flags) { + flags.bindConfigData(flagSet) + }, } lintCmdTemplate = &cmdTemplate{ @@ -342,6 +366,7 @@ $ cat input.json | prototool grpc example \ return runner.Lint(args, flags.listAllLinters, flags.listLinters) }, BindFlags: func(flagSet *pflag.FlagSet, flags *flags) { + flags.bindConfigData(flagSet) flags.bindJSON(flagSet) flags.bindListAllLinters(flagSet) flags.bindListLinters(flagSet) @@ -374,6 +399,9 @@ $ cat input.json | prototool grpc example \ Run: func(runner exec.Runner, args []string, flags *flags) error { return runner.ServiceDescriptorProto(args) }, + BindFlags: func(flagSet *pflag.FlagSet, flags *flags) { + flags.bindConfigData(flagSet) + }, } versionCmdTemplate = &cmdTemplate{ @@ -468,6 +496,12 @@ func getRunner(stdin io.Reader, stdout io.Writer, stderr io.Writer, flags *flags exec.RunnerWithCachePath(flags.cachePath), ) } + if flags.configData != "" { + runnerOptions = append( + runnerOptions, + exec.RunnerWithConfigData(flags.configData), + ) + } if flags.json { runnerOptions = append( runnerOptions, diff --git a/internal/exec/exec.go b/internal/exec/exec.go index 034d5ffd..e6149751 100644 --- a/internal/exec/exec.go +++ b/internal/exec/exec.go @@ -85,6 +85,13 @@ func RunnerWithCachePath(cachePath string) RunnerOption { } } +// RunnerWithConfigData returns a RunnerOption that uses the given config path. +func RunnerWithConfigData(configData string) RunnerOption { + return func(runner *runner) { + runner.configData = configData + } +} + // RunnerWithJSON returns a RunnerOption that will print failures as JSON. func RunnerWithJSON() RunnerOption { return func(runner *runner) { diff --git a/internal/exec/runner.go b/internal/exec/runner.go index 0190e93b..c7be9226 100644 --- a/internal/exec/runner.go +++ b/internal/exec/runner.go @@ -67,6 +67,7 @@ type runner struct { logger *zap.Logger cachePath string + configData string protocURL string printFields string json bool @@ -84,9 +85,16 @@ func newRunner(workDirPath string, input io.Reader, output io.Writer, options .. runner.configProvider = settings.NewConfigProvider( settings.ConfigProviderWithLogger(runner.logger), ) - runner.protoSetProvider = file.NewProtoSetProvider( + protoSetProviderOptions := []file.ProtoSetProviderOption{ file.ProtoSetProviderWithLogger(runner.logger), - ) + } + if runner.configData != "" { + protoSetProviderOptions = append( + protoSetProviderOptions, + file.ProtoSetProviderWithConfigData(runner.configData), + ) + } + runner.protoSetProvider = file.NewProtoSetProvider(protoSetProviderOptions...) return runner } @@ -750,6 +758,9 @@ func (r *runner) newGRPCHandler( } func (r *runner) getConfig(dirPath string) (settings.Config, error) { + if r.configData != "" { + return r.configProvider.GetForData(dirPath, r.configData) + } return r.configProvider.GetForDir(dirPath) } diff --git a/internal/file/file.go b/internal/file/file.go index afa32976..1dca60b2 100644 --- a/internal/file/file.go +++ b/internal/file/file.go @@ -119,6 +119,15 @@ func ProtoSetProviderWithLogger(logger *zap.Logger) ProtoSetProviderOption { } } +// ProtoSetProviderWithConfigData returns a ProtoSetProviderOption that uses the given configuration +// data instead of using configuration files that are found. This acts as if there is only one +// configuration file at the current working directory. All found configuration files are ignored. +func ProtoSetProviderWithConfigData(configData string) ProtoSetProviderOption { + return func(protoSetProvider *protoSetProvider) { + protoSetProvider.configData = configData + } +} + // ProtoSetProviderWithWalkTimeout returns a ProtoSetProviderOption will timeout after walking // a directory structure when searching for Protobuf files after the given amount of time. // diff --git a/internal/file/proto_set_provider.go b/internal/file/proto_set_provider.go index 96d2da44..e3ce6882 100644 --- a/internal/file/proto_set_provider.go +++ b/internal/file/proto_set_provider.go @@ -33,6 +33,7 @@ import ( type protoSetProvider struct { logger *zap.Logger + configData string walkTimeout time.Duration configProvider settings.ConfigProvider } @@ -98,23 +99,28 @@ func (c *protoSetProvider) GetMultipleForDir(workDirPath string, dirPath string) if err != nil { return nil, err } - configFilePath, err := c.configProvider.GetFilePathForDir(absDirPath) - if err != nil { - return nil, err - } - // we need everything for generation, not just the files in the given directory - // so we go back to the config file if it is shallower - // display path will be unaffected as this is based on workDirPath - configDirPath := absDirPath - if configFilePath != "" { - configDirPath = filepath.Dir(configFilePath) + // If c.configData != ", the user has specified configuration via the command line. + // Set the configuration directory to the current working directory. + configDirPath := workDirPath + if c.configData == "" { + configFilePath, err := c.configProvider.GetFilePathForDir(absDirPath) + if err != nil { + return nil, err + } + // we need everything for generation, not just the files in the given directory + // so we go back to the config file if it is shallower + // display path will be unaffected as this is based on workDirPath + configDirPath = absDirPath + if configFilePath != "" { + configDirPath = filepath.Dir(configFilePath) + } } protoFiles, err := c.walkAndGetAllProtoFiles(workDirPath, configDirPath) if err != nil { return nil, err } dirPathToProtoFiles := getDirPathToProtoFiles(protoFiles) - protoSets, err := c.getBaseProtoSets(dirPathToProtoFiles) + protoSets, err := c.getBaseProtoSets(workDirPath, dirPathToProtoFiles) if err != nil { return nil, err } @@ -136,7 +142,7 @@ func (c *protoSetProvider) GetMultipleForFiles(workDirPath string, filePaths ... return nil, err } dirPathToProtoFiles := getDirPathToProtoFiles(protoFiles) - protoSets, err := c.getBaseProtoSets(dirPathToProtoFiles) + protoSets, err := c.getBaseProtoSets(workDirPath, dirPathToProtoFiles) if err != nil { return nil, err } @@ -148,12 +154,18 @@ func (c *protoSetProvider) GetMultipleForFiles(workDirPath string, filePaths ... return protoSets, nil } -func (c *protoSetProvider) getBaseProtoSets(dirPathToProtoFiles map[string][]*ProtoFile) ([]*ProtoSet, error) { +func (c *protoSetProvider) getBaseProtoSets(absWorkDirPath string, dirPathToProtoFiles map[string][]*ProtoFile) ([]*ProtoSet, error) { filePathToProtoSet := make(map[string]*ProtoSet) for dirPath, protoFiles := range dirPathToProtoFiles { - configFilePath, err := c.configProvider.GetFilePathForDir(dirPath) - if err != nil { - return nil, err + var configFilePath string + var err error + // we only want one ProtoSet if we have set configData + // since we are overriding all configuration files + if c.configData == "" { + configFilePath, err = c.configProvider.GetFilePathForDir(dirPath) + if err != nil { + return nil, err + } } protoSet, ok := filePathToProtoSet[configFilePath] if !ok { @@ -164,8 +176,13 @@ func (c *protoSetProvider) getBaseProtoSets(dirPathToProtoFiles map[string][]*Pr } protoSet.DirPathToFiles[dirPath] = append(protoSet.DirPathToFiles[dirPath], protoFiles...) var config settings.Config - // configFilePath is empty if no config file is found - if configFilePath != "" { + if c.configData != "" { + config, err = c.configProvider.GetForData(absWorkDirPath, c.configData) + if err != nil { + return nil, err + } + } else if configFilePath != "" { + // configFilePath is empty if no config file is found config, err = c.configProvider.Get(configFilePath) if err != nil { return nil, err @@ -195,6 +212,17 @@ func (c *protoSetProvider) walkAndGetAllProtoFiles(absWorkDirPath string, absDir timedOut bool ) allExcludes := make(map[string]struct{}) + // if we have a configData, we compute the exclude prefixes once + // from this dirPath and data, and do not do it again in the below walk function + if c.configData != "" { + excludes, err := c.configProvider.GetExcludePrefixesForData(absWorkDirPath, c.configData) + if err != nil { + return nil, err + } + for _, exclude := range excludes { + allExcludes[exclude] = struct{}{} + } + } walkErrC := make(chan error) go func() { walkErrC <- filepath.Walk( @@ -212,12 +240,15 @@ func (c *protoSetProvider) walkAndGetAllProtoFiles(absWorkDirPath string, absDir // Verify if we should skip this directory/file. if fileInfo.IsDir() { // Add the excluded files with respect to the current file path. - excludes, err := c.configProvider.GetExcludePrefixesForDir(filePath) - if err != nil { - return err - } - for _, exclude := range excludes { - allExcludes[exclude] = struct{}{} + // Do not add if we have configData. + if c.configData == "" { + excludes, err := c.configProvider.GetExcludePrefixesForDir(filePath) + if err != nil { + return err + } + for _, exclude := range excludes { + allExcludes[exclude] = struct{}{} + } } if isExcluded(filePath, absDirPath, allExcludes) { return filepath.SkipDir diff --git a/internal/settings/config_provider.go b/internal/settings/config_provider.go index ddccd80b..f6c58d9f 100644 --- a/internal/settings/config_provider.go +++ b/internal/settings/config_provider.go @@ -75,6 +75,18 @@ func (c *configProvider) Get(filePath string) (Config, error) { return get(filePath) } +func (c *configProvider) GetForData(dirPath string, externalConfigData string) (Config, error) { + if !filepath.IsAbs(dirPath) { + return Config{}, fmt.Errorf("%s is not an absolute path", dirPath) + } + dirPath = filepath.Clean(dirPath) + var externalConfig ExternalConfig + if err := jsonUnmarshalStrict([]byte(externalConfigData), &externalConfig); err != nil { + return Config{}, err + } + return externalConfigToConfig(externalConfig, dirPath) +} + func (c *configProvider) GetExcludePrefixesForDir(dirPath string) ([]string, error) { if !filepath.IsAbs(dirPath) { return nil, fmt.Errorf("%s is not an absolute path", dirPath) @@ -83,6 +95,18 @@ func (c *configProvider) GetExcludePrefixesForDir(dirPath string) ([]string, err return getExcludePrefixesForDir(dirPath) } +func (c *configProvider) GetExcludePrefixesForData(dirPath string, externalConfigData string) ([]string, error) { + if !filepath.IsAbs(dirPath) { + return nil, fmt.Errorf("%s is not an absolute path", dirPath) + } + dirPath = filepath.Clean(dirPath) + var externalConfig ExternalConfig + if err := jsonUnmarshalStrict([]byte(externalConfigData), &externalConfig); err != nil { + return nil, err + } + return getExcludePrefixes(externalConfig.Excludes, dirPath) +} + // getFilePathForDir tries to find a file named by one of the ConfigFilenames starting in the // given directory, and going up a directory until hitting root. // diff --git a/internal/settings/settings.go b/internal/settings/settings.go index 5f602538..c7409d9e 100644 --- a/internal/settings/settings.go +++ b/internal/settings/settings.go @@ -311,14 +311,20 @@ type ConfigProvider interface { // If multiple files named by one of the ConfigFilenames are found in the same // directory, error is returned. GetFilePathForDir(dirPath string) (string, error) + // GetForData returns a Config for the given ExternalConfigData in JSON format. + // The Config will be as if there was a configuration file at the given dirPath. + GetForData(dirPath string, externalConfigData string) (Config, error) - // GetForDir tries to find a file named by one of the ConfigFilenames in the given + // GetExcludePrefixesForDir tries to find a file named by one of the ConfigFilenames in the given // directory and returns the cleaned absolute exclude prefixes. Unlike other functions // on ConfigProvider, this has no recursive functionality - if there is no // config file, nothing is returned. // If multiple files named by one of the ConfigFilenames are found in the same // directory, error is returned. GetExcludePrefixesForDir(dirPath string) ([]string, error) + // GetExcludePrefixesForData gets the exclude prefixes for the given ExternalConfigData in JSON format. + // The logic will act is if there was a configuration file at the given dirPath. + GetExcludePrefixesForData(dirPath string, externalConfigData string) ([]string, error) } // ConfigProviderOption is an option for a new ConfigProvider.