diff --git a/.gitignore b/.gitignore index 6dc5464f5e8..d27fcdbbb87 100644 --- a/.gitignore +++ b/.gitignore @@ -42,8 +42,6 @@ NuGet.Config nuget*.exe BenchmarkDotNet.Artifacts/ /Dockerfile -docs/gremlint/ -gremlint/ coverage.out .env gremlinconsoletest.egg-info diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 98850d85b89..349d8b2dfb9 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -27,6 +27,8 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima This release also includes changes from <>. +* Improved Gremlint formatting to keep the first argument for a step on the same line if line breaks were required to meet max line length. +* Improved Gremlint formatting to do greedy argument packing when possible so that more arguments can appear on a single line. [[release-3-8-0]] === TinkerPop 3.8.0 (Release Date: November 12, 2025) diff --git a/docs/src/reference/gremlin-applications.asciidoc b/docs/src/reference/gremlin-applications.asciidoc index d6fe2cd82f2..ca3937af9f5 100644 --- a/docs/src/reference/gremlin-applications.asciidoc +++ b/docs/src/reference/gremlin-applications.asciidoc @@ -3014,7 +3014,7 @@ describeGraph(HadoopGraph) [[gremlin-mcp]] === Gremlin MCP -Gremlin MCP integrates Apache TinkerPop with the Model Context Protocol (MCP) so that MCP‑capable assistants (for +Gremlin MCP integrates Apache TinkerPop with the Model Context Protocol (MCP) so that MCP‑capable assistants, (for example, desktop chat clients that support MCP) can discover your graph, run Gremlin traversals and exchange graph data through a small set of well‑defined tools. It allows users to “talk to your graph” while keeping full Gremlin power available when they or the assistant need it. @@ -3054,6 +3054,7 @@ The Gremlin MCP server exposes these tools: properties may be surfaced as enums to encourage valid values in queries. * `run_gremlin_query` — Executes an arbitrary Gremlin traversal and returns JSON results. * `refresh_schema_cache` — Forces schema discovery to run again when the graph has changed. +* `format_gremlin_query` — Formats a Gremlin query using gremlint. ==== Schema discovery @@ -3075,6 +3076,41 @@ Schema discovery uses Gremlin traversals and sampling to uncover the following i * Relationship patterns - Connectivity is derived from the labels of edges and their incident vertices. * Enums - Properties with a small set of distinct values may be surfaced as enumerations to promote precise filters. +==== Formatting traversals + +Gremlin is much easier to understand when it is properly formatted with appropriate line breaks and indents. An AI +assistant can format Gremlin using Gremlint via `format_gremlin_query` MCP tool which accepts any string input and +returns either a `formatted` Gremlin string or an `error` object with diagnostics. + +The formatter exposes three optional options (defaults shown): + +* `indentation` — number of spaces used for indentation (default: 0) +* `maxLineLength` — soft wrap column for lines (default: 80) +* `shouldPlaceDotsAfterLineBreaks` — when true, places method dots at the start of wrapped lines (default: false) + +For example, a user could supply this prompt to the assistant: + +[source,text] +---- +Format this Gremlin query: +``` +g.V().union(limit(3).fold(),tail(3).fold()).local( + unfold().order().by(bothE().count(),desc).limit(1).fold()) +``` +---- + +And get back: + +[source,groovy] +---- +g.V(). + union(limit(3).fold(), + tail(3).fold()). + local(unfold(). + order().by(bothE().count(), desc). + limit(1).fold()) +---- + ==== Executing traversals When the assistant needs to answer a question, a common sequence is: diff --git a/docs/src/upgrade/release-3.8.1.asciidoc b/docs/src/upgrade/release-3.8.1.asciidoc index 6da249a90d1..416f736a43b 100644 --- a/docs/src/upgrade/release-3.8.1.asciidoc +++ b/docs/src/upgrade/release-3.8.1.asciidoc @@ -23,6 +23,108 @@ complete list of all the modifications that are part of this release. === Upgrading for Users +==== Gremlint MCP + +The Gremlin MCP server now exposes Gremlint formatting for Gremlin traversals, which can be a convenient way to make +a long string of Gremlin easier to read directly from an AI assistant. Provide a simple prompt like the following to +an AI coding agent: + +[source,text] +---- +Format this Gremlin query: +``` +g.V().union(limit(3).fold(),tail(3).fold()).local( + unfold().order().by(bothE().count(),desc).limit(1).fold()) +``` +---- + +It will trigger a call to Gremlint within the Gremlin MCP Server to format it with better indentation and spacing. + +==== Gremlint Improvements + +Gremlint's approach to newlines often produced formatting that didn't generally follow the espoused best practices. +Specifically, if Gremlint found that arguments to a step would exceed the line length, it would instantly apply a +newline and then do an indent. This formatting rule made certain Gremlin appear stretched out vertically in many cases. + +The following example demonstrates this stretching. The following bit of Gremlin is manually formatted according to +common norms and best practices: + +[source,groovy] +---- +g.V().as("v"). + repeat(both().simplePath().as("v")).emit(). + filter(project("x","y","z").by(select(first, "v")). + by(select(last, "v")). + by(select(all, "v").count(local)).as("triple"). + coalesce(select("x","y").as("a"). + select("triples").unfold().as("t"). + select("x","y").where(eq("a")). + select("t"), + local(aggregate("triples"))). + select("z").as("length"). + select("triple").select("z").where(eq("length"))). + select(all, "v").unfold(). + groupCount() +---- + +In earlier versions of Gremlint, it would format that query to: + +[source,groovy] +---- +g.V().as("v"). + repeat(both().simplePath().as("v")).emit(). + filter( + project("x", "y", "z"). + by(select(first, "v")). + by(select(last, "v")). + by(select(all, "v").count(local)). + as("triple"). + coalesce( + select("x", "y").as("a"). + select("triples"). + unfold().as("t"). + select("x", "y"). + where(eq("a")). + select("t"), + local(aggregate("triples"))). + select("z").as("length"). + select("triple"). + select("z"). + where(eq("length"))). + select(all, "v"). + unfold(). + groupCount() +---- + +In this version, after the improvements mentioned above, Gremlint now produces: + +[source,groovy] +---- +g.V().as("v"). + repeat(both().simplePath().as("v")).emit(). + filter(project("x", "y", "z"). + by(select(first, "v")). + by(select(last, "v")). + by(select(all, "v").count(local)). + as("triple"). + coalesce(select("x", "y").as("a"). + select("triples"). + unfold().as("t"). + select("x", "y"). + where(eq("a")). + select("t"), local(aggregate("triples"))). + select("z").as("length"). + select("triple"). + select("z"). + where(eq("length"))). + select(all, "v"). + unfold(). + groupCount() +---- + +This more compact representation presents a form much more in line with the manually formatted one. While there is still +room to improve, Gremlint now produces a format that is more likely to be usable without additional manual formatting +intervention === Upgrading for Providers diff --git a/gremlin-mcp/src/main/javascript/README.md b/gremlin-mcp/src/main/javascript/README.md index 5b6f619aad2..aa2a6659d3f 100644 --- a/gremlin-mcp/src/main/javascript/README.md +++ b/gremlin-mcp/src/main/javascript/README.md @@ -48,6 +48,7 @@ Your AI assistant gets access to these powerful tools: | 📋 **get_graph_schema** | Schema Discovery | Get complete graph structure with vertices and edges | | ⚡ **run_gremlin_query** | Query Execution | Execute any Gremlin traversal query with full syntax support | | 🔄 **refresh_schema_cache** | Cache Management | Force immediate refresh of cached schema information | +| 👌 **format_gremlin_query** | Query Formatting | Format a Gremlin query string using gremlint | ## 🚀 Quick Setup @@ -171,6 +172,12 @@ Restart your AI client and try asking: One of the most powerful features of this MCP server is **Automatic Enum Discovery** - it intelligently analyzes your graph data to discover valid property values and provides them as enums to AI agents. +### Query Formatting + +**You ask:** _"Format this Gremlin query \`g.V().out('both').project('name','age').by('name').by('age')\`."_ + +**AI response:** The AI calls the `format_gremlin_query` tool and returns a formatted Gremlin string (or a structured error if parsing fails). Optional formatting options include `indentation`, `maxLineLength`, and `shouldPlaceDotsAfterLineBreaks`. + ### 🤔 The Problem It Solves **Without Enum Discovery:** diff --git a/gremlin-mcp/src/main/javascript/package-lock.json b/gremlin-mcp/src/main/javascript/package-lock.json index 26344967c79..bb72a395b02 100644 --- a/gremlin-mcp/src/main/javascript/package-lock.json +++ b/gremlin-mcp/src/main/javascript/package-lock.json @@ -16,6 +16,7 @@ "@types/gremlin": "^3.6.7", "effect": "^3.17.9", "gremlin": "^3.7.4", + "gremlint": "^3.8.0", "winston": "^3.17.0", "zod": "^3.25.76" }, @@ -5145,6 +5146,14 @@ "uuid": "dist/bin/uuid" } }, + "node_modules/gremlint": { + "version": "3.8.0", + "resolved": "https://registry.npmjs.org/gremlint/-/gremlint-3.8.0.tgz", + "integrity": "sha512-9R3KXzhqz2DpmjAdQ+hadjnwEg4ubVYEdGUClCbG4xGXrqo8GxwnLPQWLCQ9NTkcOdXXlzuDaiYKRL1XJoLF4w==", + "engines": { + "node": ">=20" + } + }, "node_modules/handlebars": { "version": "4.7.8", "resolved": "https://registry.npmjs.org/handlebars/-/handlebars-4.7.8.tgz", diff --git a/gremlin-mcp/src/main/javascript/package.json b/gremlin-mcp/src/main/javascript/package.json index 60e564261c9..4e9fe9b0465 100644 --- a/gremlin-mcp/src/main/javascript/package.json +++ b/gremlin-mcp/src/main/javascript/package.json @@ -42,6 +42,7 @@ }, "license": "Apache-2.0", "dependencies": { + "gremlint": "^3.8.0", "@effect/platform": "^0.90.6", "@effect/platform-node": "^0.96.0", "@modelcontextprotocol/sdk": "^1.17.4", diff --git a/gremlin-mcp/src/main/javascript/src/constants.ts b/gremlin-mcp/src/main/javascript/src/constants.ts index 061cf8833ba..bb65a0e134d 100644 --- a/gremlin-mcp/src/main/javascript/src/constants.ts +++ b/gremlin-mcp/src/main/javascript/src/constants.ts @@ -9,7 +9,7 @@ * * http://www.apache.org/licenses/LICENSE-2.0 * - * Unless required by applicable law or agreed to in writing, + * Unless required by applicable law or agreed in writing, * software distributed under the License is distributed on an * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * KIND, either express or implied. See the License for the @@ -44,6 +44,7 @@ export const TOOL_NAMES = { GET_GRAPH_SCHEMA: 'get_graph_schema', RUN_GREMLIN_QUERY: 'run_gremlin_query', REFRESH_SCHEMA_CACHE: 'refresh_schema_cache', + FORMAT_GREMLIN_QUERY: 'format_gremlin_query', } as const; // Default Configuration Values diff --git a/gremlin-mcp/src/main/javascript/src/handlers/tools.ts b/gremlin-mcp/src/main/javascript/src/handlers/tools.ts index 0c6909bc78b..bb143804fc1 100644 --- a/gremlin-mcp/src/main/javascript/src/handlers/tools.ts +++ b/gremlin-mcp/src/main/javascript/src/handlers/tools.ts @@ -9,7 +9,7 @@ * * http://www.apache.org/licenses/LICENSE-2.0 * - * Unless required by applicable law or agreed to in writing, + * Unless required by applicable law or agreed in writing, * software distributed under the License is distributed on an * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * KIND, either express or implied. See the License for the @@ -30,7 +30,13 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { z } from 'zod'; import { TOOL_NAMES } from '../constants.js'; import { GremlinService } from '../gremlin/service.js'; -import { createToolEffect, createStringToolEffect, createQueryEffect } from './tool-patterns.js'; +import { + createToolEffect, + createStringToolEffect, + createQueryEffect, + createSuccessResponse, +} from './tool-patterns.js'; +import { formatQuery } from 'gremlint'; /** * Input validation schemas for tool parameters. @@ -51,6 +57,17 @@ const runQueryInputSchema = z.object({ .describe('The Gremlin query to execute'), }); +// Format Gremlin Query input - allow any string and expose gremlint options as top-level optional fields +const formatQueryInputSchema = z + .object({ + query: z.string().describe('The Gremlin query (or any string) to format'), + // Expose gremlint options as optional top-level fields + indentation: z.number().int().nonnegative().optional(), + maxLineLength: z.number().int().positive().optional(), + shouldPlaceDotsAfterLineBreaks: z.boolean().optional(), + }) + .strict(); + /** * Registers all MCP tool handlers with the server. * @@ -144,4 +161,52 @@ export function registerEffectToolHandlers( return Effect.runPromise(pipe(createQueryEffect(query), Effect.provide(runtime))); } ); + + // Format Gremlin Query (uses local gremlint) + server.registerTool( + TOOL_NAMES.FORMAT_GREMLIN_QUERY, + { + title: 'Format Gremlin Query', + description: 'Format a Gremlin query using Gremlint and return a structured result', + inputSchema: formatQueryInputSchema.shape, + }, + (args: unknown) => { + const parsed = formatQueryInputSchema.parse(args); + const { query, indentation, maxLineLength, shouldPlaceDotsAfterLineBreaks } = parsed; + + // Build options only with fields provided (undefineds will be ignored by gremlint defaults) + const options = + indentation !== undefined || + maxLineLength !== undefined || + shouldPlaceDotsAfterLineBreaks !== undefined + ? { indentation, maxLineLength, shouldPlaceDotsAfterLineBreaks } + : undefined; + + const effect = Effect.try(() => formatQuery(query, options)); + + // Map success to structured JSON and errors to structured error object (still returned as success response) + const responseEffect = pipe( + effect, + Effect.map(formatted => + createSuccessResponse({ success: true, formattedQuery: formatted }) + ), + Effect.catchAll(error => + Effect.succeed( + createSuccessResponse({ + success: false, + error: { + message: String(error), + // include common error fields when present to make it structured + name: (error && (error as any).name) || undefined, + stack: (error && (error as any).stack) || undefined, + details: (error && (error as any).details) || undefined, + }, + }) + ) + ) + ); + + return Effect.runPromise(pipe(responseEffect, Effect.provide(runtime))); + } + ); } diff --git a/gremlin-mcp/src/main/javascript/tests/gremlint-format.test.ts b/gremlin-mcp/src/main/javascript/tests/gremlint-format.test.ts new file mode 100644 index 00000000000..1bdb46d3830 --- /dev/null +++ b/gremlin-mcp/src/main/javascript/tests/gremlint-format.test.ts @@ -0,0 +1,66 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { describe, it, expect, beforeAll } from '@jest/globals'; + +let formatQuery: (q: string, opts?: any) => string; + +beforeAll(async () => { + // Import the compiled gremlint artifact directly from the repository (lib/index.js) + const mod = await import('../../../../../gremlint/lib/index.js'); + // Resolve across possible interop shapes + formatQuery = + (mod && (mod.formatQuery as any)) || + (mod && mod.default && (mod.default.formatQuery as any)) || + (mod && (mod.default as any))?.formatQuery || + (mod as any); + + if (!formatQuery || typeof formatQuery !== 'function') { + throw new Error( + 'Could not resolve formatQuery from gremlint module. Please build gremlint first.' + ); + } +}); + +describe('gremlint formatQuery', () => { + const sample = `g.V().hasLabel('person').has('name','marko').out('knows').values('name')`; + + it('formats and returns a different string that contains newlines', () => { + const formatted = formatQuery(sample, { + indentation: 2, + maxLineLength: 60, + shouldPlaceDotsAfterLineBreaks: true, + }); + expect(typeof formatted).toBe('string'); + expect(formatted).not.toBe(sample); + expect(formatted.includes('\n')).toBe(true); + }); + + it('produces different output for different indentation options', () => { + const f2 = formatQuery(sample, { indentation: 2 }); + const f4 = formatQuery(sample, { indentation: 4 }); + expect(f2).not.toBe(f4); + + const secondLine2 = f2.split('\n')[1] || ''; + const secondLine4 = f4.split('\n')[1] || ''; + + const leadingSpaces = (s: string) => s.match(/^ */)?.[0].length || 0; + expect(leadingSpaces(secondLine2)).toBeLessThanOrEqual(leadingSpaces(secondLine4)); + }); +}); diff --git a/gremlint/src/formatQuery/__tests__/closureIndentation.test.ts b/gremlint/src/formatQuery/__tests__/closureIndentation.test.ts index f0aaadfde90..43326fb31ad 100644 --- a/gremlint/src/formatQuery/__tests__/closureIndentation.test.ts +++ b/gremlint/src/formatQuery/__tests__/closureIndentation.test.ts @@ -79,11 +79,10 @@ by{ it.get().value('sell_price') - ), ).toBe( `g.V(). - filter( - out('Sells'). - map{ it.get('sell_price') - - it.get('buy_price') }. - where(gt(50)))`, + filter(out('Sells'). + map{ it.get('sell_price') - + it.get('buy_price') }. + where(gt(50)))`, ); expect( @@ -115,10 +114,9 @@ by{ it.get().value('sell_price') - { indentation: 0, maxLineLength: 22, shouldPlaceDotsAfterLineBreaks: false }, ), ).toBe(`g.V(). - filter( - map{ one = 1 - two = 2 - three = 3 }))`); + filter(map{ one = 1 + two = 2 + three = 3 }))`); expect( formatQuery( @@ -149,10 +147,10 @@ by{ it.get().value('sell_price') - { indentation: 0, maxLineLength: 45, shouldPlaceDotsAfterLineBreaks: false }, ), ).toBe(`g.V(). - where( - map{ buyPrice = it.get().value('buy_price'); - sellPrice = it.get().value('sell_price'); - sellPrice - buyPrice; }.is(gt(50)))`); + where(map{ buyPrice = it.get().value('buy_price'); + sellPrice = it.get().value('sell_price'); + sellPrice - buyPrice; }. + is(gt(50)))`); expect( formatQuery( @@ -183,10 +181,11 @@ by{ it.get().value('sell_price') - { indentation: 0, maxLineLength: 50, shouldPlaceDotsAfterLineBreaks: false }, ), ).toBe(`g.V(). - where( - out().map{ buyPrice = it.get().value('buy_price'); - sellPrice = it.get().value('sell_price'); - sellPrice - buyPrice; }.is(gt(50)))`); + where(out(). + map{ buyPrice = it.get().value('buy_price'); + sellPrice = it.get().value('sell_price'); + sellPrice - buyPrice; }. + is(gt(50)))`); expect( formatQuery( `g.V().where(out().map{ buyPrice = it.get().value('buy_price'); @@ -195,12 +194,11 @@ by{ it.get().value('sell_price') - { indentation: 0, maxLineLength: 45, shouldPlaceDotsAfterLineBreaks: false }, ), ).toBe(`g.V(). - where( - out(). - map{ buyPrice = it.get().value('buy_price'); - sellPrice = it.get().value('sell_price'); - sellPrice - buyPrice; }. - is(gt(50)))`); + where(out(). + map{ buyPrice = it.get().value('buy_price'); + sellPrice = it.get().value('sell_price'); + sellPrice - buyPrice; }. + is(gt(50)))`); // Test that relative indentation is preserved between all the lines within a closure when not all tokens in a stepGroup are methods (for instance, g in g.V() adds to the width of the stepGroup even if it is not a method) expect( @@ -246,15 +244,12 @@ by{ it.get().value('sell_price') - ).toBe(`g.V(ids). has('factor_a'). has('factor_b'). - project( - 'Factor A', - 'Factor B', - 'Product'). + project('Factor A', 'Factor B', + 'Product'). by(values('factor_a')). by(values('factor_b')). - by( - map{ it.get().value('factor_a') * - it.get().value('factor_b') })`); + by(map{ it.get().value('factor_a') * + it.get().value('factor_b') })`); // Test that relative indentation is preserved between all lines within a closure when dots are placed after line breaks // When the whole query is long enough to wrap @@ -300,12 +295,11 @@ by{ it.get().value('sell_price') - { indentation: 0, maxLineLength: 45, shouldPlaceDotsAfterLineBreaks: true }, ), ).toBe(`g.V() - .where( - out() - .map{ buyPrice = it.get().value('buy_price'); - sellPrice = it.get().value('sell_price'); - sellPrice - buyPrice; } - .is(gt(50)))`); + .where(out() + .map{ buyPrice = it.get().value('buy_price'); + sellPrice = it.get().value('sell_price'); + sellPrice - buyPrice; } + .is(gt(50)))`); // When the query is long enough to wrap, but the traversal containing the closure is the first step in its traversal and not long enough to wrap expect( @@ -323,15 +317,12 @@ by{ it.get().value('sell_price') - ).toBe(`g.V(ids) .has('factor_a') .has('factor_b') - .project( - 'Factor A', - 'Factor B', - 'Product') + .project('Factor A', 'Factor B', + 'Product') .by(values('factor_a')) .by(values('factor_b')) - .by( - map{ it.get().value('factor_a') * - it.get().value('factor_b') })`); + .by(map{ it.get().value('factor_a') * + it.get().value('factor_b') })`); // When the whole query is short enough to not wrap expect( diff --git a/gremlint/src/formatQuery/__tests__/defaultConfig.test.ts b/gremlint/src/formatQuery/__tests__/defaultConfig.test.ts index c6ace66189c..2247b17903b 100644 --- a/gremlint/src/formatQuery/__tests__/defaultConfig.test.ts +++ b/gremlint/src/formatQuery/__tests__/defaultConfig.test.ts @@ -49,9 +49,8 @@ test('It should be possible ot use formatQuery with a config, with a partial con ).toBe(` g.V() .has('person', 'name', 'marko') .shortestPath() - .with( - ShortestPath.target, - __.has('name', 'josh')) + .with(ShortestPath.target, + __.has('name', 'josh')) .with(ShortestPath.distance, 'weight')`); // Test using formatQuery with an empty config diff --git a/gremlint/src/formatQuery/__tests__/determineWhatPartsOfCodeAreGremlin.test.ts b/gremlint/src/formatQuery/__tests__/determineWhatPartsOfCodeAreGremlin.test.ts index 3f1a2e66e12..2123607fda5 100644 --- a/gremlint/src/formatQuery/__tests__/determineWhatPartsOfCodeAreGremlin.test.ts +++ b/gremlint/src/formatQuery/__tests__/determineWhatPartsOfCodeAreGremlin.test.ts @@ -38,9 +38,8 @@ g.V().filter(values('name').filter(contains('Gremlint')))`, } g.V(). - filter( - values('name'). - filter(contains('Gremlint')))`); + filter(values('name'). + filter(contains('Gremlint')))`); expect( formatQuery( diff --git a/gremlint/src/formatQuery/__tests__/dotsAfterLineBreaks.test.ts b/gremlint/src/formatQuery/__tests__/dotsAfterLineBreaks.test.ts index 831e1b84795..4c947f02026 100644 --- a/gremlint/src/formatQuery/__tests__/dotsAfterLineBreaks.test.ts +++ b/gremlint/src/formatQuery/__tests__/dotsAfterLineBreaks.test.ts @@ -34,10 +34,9 @@ test('If dots are configured to be placed after line breaks, make sure they are group(). by(values('name', 'age').fold()). unfold(). - filter( - select(values). - count(local). - is(gt(1)))`); + filter(select(values). + count(local). + is(gt(1)))`); expect( formatQuery( @@ -51,11 +50,10 @@ test('If dots are configured to be placed after line breaks, make sure they are ).toBe(`g.V() .hasLabel('person') .group() - .by( - values('name', 'age').fold()) + .by(values('name', 'age') + .fold()) .unfold() - .filter( - select(values) - .count(local) - .is(gt(1)))`); + .filter(select(values) + .count(local) + .is(gt(1)))`); }); diff --git a/gremlint/src/formatQuery/__tests__/invalidIndentationAndMaxLineLength.test.ts b/gremlint/src/formatQuery/__tests__/invalidIndentationAndMaxLineLength.test.ts index e743a11b0de..0aed02e7ea1 100644 --- a/gremlint/src/formatQuery/__tests__/invalidIndentationAndMaxLineLength.test.ts +++ b/gremlint/src/formatQuery/__tests__/invalidIndentationAndMaxLineLength.test.ts @@ -26,6 +26,5 @@ test('The formatter should not crash when indentation is equal to maxLineLength' maxLineLength: 0, shouldPlaceDotsAfterLineBreaks: false, }), - ).toBe(`g.V( -)`); + ).toBe(`g.V()`); }); diff --git a/gremlint/src/formatQuery/__tests__/layoutUtils.test.ts b/gremlint/src/formatQuery/__tests__/layoutUtils.test.ts new file mode 100644 index 00000000000..af14c0c164c --- /dev/null +++ b/gremlint/src/formatQuery/__tests__/layoutUtils.test.ts @@ -0,0 +1,68 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { willFitOnLine, getOneLinerWidth } from '../layoutUtils'; +import { TokenType, UnformattedSyntaxTree } from '../types'; + +describe('layoutUtils', () => { + const config = { + globalIndentation: 0, + maxLineLength: 20, + }; + + const simpleWord: UnformattedSyntaxTree = { + type: TokenType.Word, + word: 'g.V()', + }; + + const longWord: UnformattedSyntaxTree = { + type: TokenType.Word, + word: 'veryLongTraversalDefinition', + }; + + describe('willFitOnLine', () => { + test('should return true if query fits', () => { + expect(willFitOnLine(simpleWord, config)).toBe(true); + }); + + test('should return false if query exceeds maxLineLength', () => { + expect(willFitOnLine(longWord, config)).toBe(false); + }); + + test('should account for global indentation', () => { + const indentedConfig = { ...config, globalIndentation: 16 }; + expect(willFitOnLine(simpleWord, indentedConfig)).toBe(false); + }); + + test('should account for horizontal position', () => { + expect(willFitOnLine(simpleWord, config, 16)).toBe(false); + }); + }); + + describe('getOneLinerWidth', () => { + test('should return the correct width', () => { + expect(getOneLinerWidth(simpleWord)).toBe(5); + }); + + test('should account for horizontal position in width if applicable (though usually trimmed)', () => { + // getOneLinerWidth trims the result + expect(getOneLinerWidth(simpleWord, 10)).toBe(5); + }); + }); +}); diff --git a/gremlint/src/formatQuery/__tests__/modulatorIndentation.test.ts b/gremlint/src/formatQuery/__tests__/modulatorIndentation.test.ts index 4ebb342921c..8357b409bc9 100644 --- a/gremlint/src/formatQuery/__tests__/modulatorIndentation.test.ts +++ b/gremlint/src/formatQuery/__tests__/modulatorIndentation.test.ts @@ -71,10 +71,9 @@ test('Wrapped modulators should be indented with two spaces', () => { group(). by(values('name', 'age').fold()). unfold(). - filter( - select(values). - count(local). - is(gt(1)))`); + filter(select(values). + count(local). + is(gt(1)))`); expect( formatQuery( "g.V().hasLabel('person').groupCount().by(values('age').choose(is(lt(28)),constant('young'),choose(is(lt(30)), constant('old'), constant('very old'))))", @@ -87,12 +86,9 @@ test('Wrapped modulators should be indented with two spaces', () => { ).toBe(`g.V(). hasLabel('person'). groupCount(). - by( - values('age'). - choose( - is(lt(28)), - constant('young'), - choose(is(lt(30)), constant('old'), constant('very old'))))`); + by(values('age'). + choose(is(lt(28)), constant('young'), + choose(is(lt(30)), constant('old'), constant('very old'))))`); // Test emit()-modulator indentation expect( @@ -161,10 +157,9 @@ test('Wrapped modulators should be indented with two spaces', () => { ), ).toBe( `g.V(). - has( - 'person', - 'name', - 'vadas'). + has('person', + 'name', + 'vadas'). as('e'). in('knows'). as('m'). @@ -187,10 +182,9 @@ test('Wrapped modulators should be indented with two spaces', () => { ), ).toBe( `g.V(). - has( - 'person', - 'name', - 'vadas'). + has('person', + 'name', + 'vadas'). as_('e'). in('knows'). as_('m'). @@ -260,9 +254,8 @@ test('Wrapped modulators should be indented with two spaces', () => { `g.V(v1). addE('knows'). to(v2). - property( - 'weight', - 0.75). + property('weight', + 0.75). iterate()`, ); @@ -279,12 +272,10 @@ test('Wrapped modulators should be indented with two spaces', () => { ).toBe( `g.V(6). repeat('a', both('created').simplePath()). - emit( - repeat('b', both('knows')). - until( - loops('b').as('b'). - where(loops('a').as('b'))). - hasId(2)). + emit(repeat('b', both('knows')). + until(loops('b').as('b'). + where(loops('a').as('b'))). + hasId(2)). dedup()`, ); @@ -501,15 +492,14 @@ test('Wrapped modulators should be indented with two spaces', () => { }, ), ).toBe( - `g.inject( - g.withComputer(). - V(). - shortestPath(). - with(ShortestPath.distance, 'weight'). - with(ShortestPath.includeEdges, true). - with(ShortestPath.maxDistance, 1). - toList(). - toArray()). + `g.inject(g.withComputer(). + V(). + shortestPath(). + with(ShortestPath.distance, 'weight'). + with(ShortestPath.includeEdges, true). + with(ShortestPath.maxDistance, 1). + toList(). + toArray()). map(unfold().values('name', 'weight').fold())`, ); expect( @@ -534,9 +524,8 @@ test('Wrapped modulators should be indented with two spaces', () => { `g.V(). hasLabel('person'). valueMap('name'). - with( - WithOptions.tokens, - WithOptions.labels)`, + with(WithOptions.tokens, + WithOptions.labels)`, ); expect( formatQuery( @@ -552,9 +541,8 @@ test('Wrapped modulators should be indented with two spaces', () => { hasLabel('person'). properties('location'). valueMap(). - with( - WithOptions.tokens, - WithOptions.values)`, + with(WithOptions.tokens, + WithOptions.values)`, ); // Test with_()-modulator indentation @@ -760,6 +748,26 @@ test('Wrapped modulators should be indented with two spaces', () => { with_(ShortestPath.target, __.has('name', 'josh')). with_(ShortestPath.includeEdges, true)`, ); + expect( + formatQuery( + "g.inject(g.withComputer().V().shortestPath().with_(ShortestPath.distance, 'weight').with_(ShortestPath.includeEdges, true).with_(ShortestPath.maxDistance, 1).toList().toArray()).map(unfold().values('name','weight').fold())", + { + indentation: 0, + maxLineLength: 52, + shouldPlaceDotsAfterLineBreaks: false, + }, + ), + ).toBe( + `g.inject(g.withComputer(). + V(). + shortestPath(). + with_(ShortestPath.distance, 'weight'). + with_(ShortestPath.includeEdges, true). + with_(ShortestPath.maxDistance, 1). + toList(). + toArray()). + map(unfold().values('name', 'weight').fold())`, + ); expect( formatQuery( "g.inject(g.withComputer().V().shortestPath().with_(ShortestPath.distance, 'weight').with_(ShortestPath.includeEdges, true).with_(ShortestPath.maxDistance, 1).toList().toArray()).map(unfold().values('name','weight').fold())", @@ -770,15 +778,15 @@ test('Wrapped modulators should be indented with two spaces', () => { }, ), ).toBe( - `g.inject( - g.withComputer(). - V(). - shortestPath(). - with_(ShortestPath.distance, 'weight'). - with_(ShortestPath.includeEdges, true). - with_(ShortestPath.maxDistance, 1). - toList(). - toArray()). + `g.inject(g.withComputer(). + V(). + shortestPath(). + with_(ShortestPath.distance, + 'weight'). + with_(ShortestPath.includeEdges, true). + with_(ShortestPath.maxDistance, 1). + toList(). + toArray()). map(unfold().values('name', 'weight').fold())`, ); expect( @@ -803,9 +811,8 @@ test('Wrapped modulators should be indented with two spaces', () => { `g.V(). hasLabel('person'). valueMap('name'). - with_( - WithOptions.tokens, - WithOptions.labels)`, + with_(WithOptions.tokens, + WithOptions.labels)`, ); expect( formatQuery( @@ -821,9 +828,8 @@ test('Wrapped modulators should be indented with two spaces', () => { hasLabel('person'). properties('location'). valueMap(). - with_( - WithOptions.tokens, - WithOptions.values)`, + with_(WithOptions.tokens, + WithOptions.values)`, ); // Test write()-modulator indentation @@ -838,4 +844,24 @@ test('Wrapped modulators should be indented with two spaces', () => { write(). iterate()`, ); + + + expect( + formatQuery( + `g.V(ids). + has('factor_a'). + has('factor_b'). + project('Factor A', 'Factor B', 'Product'). + by(values('factor_a')). + by(values('factor_b')). + by(__.out('knows').in('created').has('product','code','xXz-343-94985AD'))`, + { indentation: 0, maxLineLength: 79, shouldPlaceDotsAfterLineBreaks: false }, + ), + ).toBe(`g.V(ids). + has('factor_a'). + has('factor_b'). + project('Factor A', 'Factor B', 'Product'). + by(values('factor_a')). + by(values('factor_b')). + by(__.out('knows').in('created').has('product', 'code', 'xXz-343-94985AD'))`); }); diff --git a/gremlint/src/formatQuery/__tests__/modulatorWrapping.test.ts b/gremlint/src/formatQuery/__tests__/modulatorWrapping.test.ts index 8859eac95a3..5a8198bad26 100644 --- a/gremlint/src/formatQuery/__tests__/modulatorWrapping.test.ts +++ b/gremlint/src/formatQuery/__tests__/modulatorWrapping.test.ts @@ -105,8 +105,9 @@ test("Modulators should not be line-wrapped if they can fit on the line of the s hasLabel('person'). group().by(values('name', 'age').fold()). unfold(). - filter( - select(values).count(local).is(gt(1)))`); + filter(select(values). + count(local). + is(gt(1)))`); expect( formatQuery( "g.V().hasLabel('person').group().by(values('name', 'age').fold()).unfold().filter(select(values).count(local).is(gt(1)))", @@ -121,8 +122,7 @@ test("Modulators should not be line-wrapped if they can fit on the line of the s group(). by(values('name', 'age').fold()). unfold(). - filter( - select(values). - count(local). - is(gt(1)))`); + filter(select(values). + count(local). + is(gt(1)))`); }); diff --git a/gremlint/src/formatQuery/formatSyntaxTrees/formatClosure.ts b/gremlint/src/formatQuery/formatSyntaxTrees/formatClosure.ts index 9bfd023f488..2a7aa703ff8 100644 --- a/gremlint/src/formatQuery/formatSyntaxTrees/formatClosure.ts +++ b/gremlint/src/formatQuery/formatSyntaxTrees/formatClosure.ts @@ -28,50 +28,75 @@ import { UnformattedClosureSyntaxTree, } from '../types'; import { withNoEndDotInfo } from './utils'; +import { getOneLinerWidth, willFitOnLine } from '../layoutUtils'; const getClosureLineOfCodeIndentation = ( relativeIndentation: number, horizontalPosition: number, methodWidth: number, lineNumber: number, + shouldStartWithDot: boolean, ) => { if (lineNumber === 0) return Math.max(relativeIndentation, 0); - return Math.max(relativeIndentation + horizontalPosition + methodWidth + 1, 0); + return Math.max( + relativeIndentation + horizontalPosition + methodWidth + (shouldStartWithDot ? 1 : 0) + 1, + 0, + ); }; -const getFormattedClosureLineOfCode = (horizontalPosition: number, methodWidth: number) => ( - { lineOfCode, relativeIndentation }: UnformattedClosureLineOfCode, - lineNumber: number, -) => ({ +const getFormattedClosureLineOfCode = ( + horizontalPosition: number, + methodWidth: number, + shouldStartWithDot: boolean, +) => ({ lineOfCode, relativeIndentation }: UnformattedClosureLineOfCode, lineNumber: number) => ({ lineOfCode, relativeIndentation, - localIndentation: getClosureLineOfCodeIndentation(relativeIndentation, horizontalPosition, methodWidth, lineNumber), + localIndentation: getClosureLineOfCodeIndentation( + relativeIndentation, + horizontalPosition, + methodWidth, + lineNumber, + shouldStartWithDot, + ), }); const getFormattedClosureCodeBlock = ( unformattedClosureCodeBlock: UnformattedClosureCodeBlock, horizontalPosition: number, methodWidth: number, + shouldStartWithDot: boolean, ) => { - return unformattedClosureCodeBlock.map(getFormattedClosureLineOfCode(horizontalPosition, methodWidth)); + return unformattedClosureCodeBlock.map( + getFormattedClosureLineOfCode(horizontalPosition, methodWidth, shouldStartWithDot), + ); }; export const formatClosure = (formatSyntaxTree: GremlinSyntaxTreeFormatter) => (config: GremlintInternalConfig) => ( syntaxTree: UnformattedClosureSyntaxTree, ): FormattedClosureSyntaxTree => { const { closureCodeBlock: unformattedClosureCodeBlock, method: unformattedMethod } = syntaxTree; - const { localIndentation, horizontalPosition, maxLineLength, shouldEndWithDot } = config; - const recreatedQuery = recreateQueryOnelinerFromSyntaxTree(localIndentation)(syntaxTree); + const { + localIndentation, + horizontalPosition, + maxLineLength, + shouldEndWithDot, + shouldStartWithDot, + } = config; const formattedMethod = formatSyntaxTree(withNoEndDotInfo(config))(unformattedMethod); const methodWidth = formattedMethod.width; - if (recreatedQuery.length <= maxLineLength) { + if (willFitOnLine(syntaxTree, config, horizontalPosition)) { return { type: TokenType.Closure, method: formattedMethod, - closureCodeBlock: getFormattedClosureCodeBlock(unformattedClosureCodeBlock, horizontalPosition, methodWidth), + closureCodeBlock: getFormattedClosureCodeBlock( + unformattedClosureCodeBlock, + horizontalPosition, + methodWidth, + shouldStartWithDot, + ), localIndentation, - width: recreatedQuery.trim().length, + width: getOneLinerWidth(syntaxTree, horizontalPosition), shouldStartWithDot: false, shouldEndWithDot: Boolean(shouldEndWithDot), }; @@ -80,7 +105,12 @@ export const formatClosure = (formatSyntaxTree: GremlinSyntaxTreeFormatter) => ( return { type: TokenType.Closure, method: formattedMethod, - closureCodeBlock: getFormattedClosureCodeBlock(unformattedClosureCodeBlock, horizontalPosition, methodWidth), + closureCodeBlock: getFormattedClosureCodeBlock( + unformattedClosureCodeBlock, + horizontalPosition, + methodWidth, + shouldStartWithDot, + ), localIndentation: 0, width: 0, shouldStartWithDot: false, diff --git a/gremlint/src/formatQuery/formatSyntaxTrees/formatMethod.ts b/gremlint/src/formatQuery/formatSyntaxTrees/formatMethod.ts index fd4f92b4ffa..5a2af979426 100644 --- a/gremlint/src/formatQuery/formatSyntaxTrees/formatMethod.ts +++ b/gremlint/src/formatQuery/formatSyntaxTrees/formatMethod.ts @@ -27,10 +27,12 @@ import { UnformattedMethodSyntaxTree, } from '../types'; import { last, pipe, sum } from '../utils'; +import { willFitOnLine } from '../layoutUtils'; import { withHorizontalPosition, withIncreasedHorizontalPosition, withIncreasedIndentation, + withIndentation, withNoEndDotInfo, withZeroDotInfo, withZeroIndentation, @@ -40,10 +42,10 @@ import { export const formatMethod = (formatSyntaxTree: GremlinSyntaxTreeFormatter) => (config: GremlintInternalConfig) => ( syntaxTree: UnformattedMethodSyntaxTree, ): FormattedMethodSyntaxTree => { - const recreatedQuery = recreateQueryOnelinerFromSyntaxTree(config.localIndentation)(syntaxTree); const method = formatSyntaxTree(withNoEndDotInfo(config))(syntaxTree.method); - const argumentsWillNotBeWrapped = recreatedQuery.length <= config.maxLineLength; + const argumentsWillNotBeWrapped = willFitOnLine(syntaxTree, config, config.horizontalPosition); if (argumentsWillNotBeWrapped) { + const recreatedQuery = recreateQueryOnelinerFromSyntaxTree(config, config.horizontalPosition)(syntaxTree); return { type: TokenType.Method, method, @@ -61,7 +63,11 @@ export const formatMethod = (formatSyntaxTree: GremlinSyntaxTreeFormatter) => (c withZeroIndentation, withZeroDotInfo, withIncreasedHorizontalPosition( - method.width + 1 + argumentGroup.map(({ width }) => width).reduce(sum, 0) + argumentGroup.length, + method.width + + 1 + + (config.shouldStartWithDot ? 1 : 0) + + argumentGroup.map(({ width }) => width).reduce(sum, 0) + + argumentGroup.length, ), )(config), )(syntaxTree), @@ -69,34 +75,108 @@ export const formatMethod = (formatSyntaxTree: GremlinSyntaxTreeFormatter) => (c }, []), ], argumentsShouldStartOnNewLine: false, - localIndentation: config.localIndentation, shouldStartWithDot: false, shouldEndWithDot: Boolean(config.shouldEndWithDot), + localIndentation: config.localIndentation, + horizontalPosition: config.horizontalPosition, width: recreatedQuery.trim().length, }; } // shouldEndWithDot has to reside on the method object, so the end dot can be // placed after the method parentheses. shouldStartWithDot has to be passed on // further down so the start dot can be placed after the indentation. - const argumentGroups = syntaxTree.arguments.map((step) => [ + const horizontalPosition = config.horizontalPosition + method.width + (config.shouldStartWithDot ? 1 : 0) + 1; + + const greedyArgumentGroups = syntaxTree.arguments.reduce( + (acc: { groups: FormattedSyntaxTree[][]; currentGroup: FormattedSyntaxTree[] }, arg) => { + const formattedArg = formatSyntaxTree( + pipe(withZeroIndentation, withZeroDotInfo, withHorizontalPosition(horizontalPosition))(config), + )(arg); + const currentGroupWidth = + acc.currentGroup.map(({ width }) => width).reduce(sum, 0) + Math.max(acc.currentGroup.length - 1, 0) * 2; + const potentialGroupWidth = + acc.currentGroup.length > 0 ? currentGroupWidth + 2 + formattedArg.width : formattedArg.width; + + if (acc.currentGroup.length > 0 && horizontalPosition + potentialGroupWidth > config.maxLineLength) { + return { + groups: [...acc.groups, acc.currentGroup], + currentGroup: [ + formatSyntaxTree( + pipe(withIndentation(horizontalPosition), withZeroDotInfo, withHorizontalPosition(horizontalPosition))( + config, + ), + )(arg), + ], + }; + } + return { + ...acc, + currentGroup: [ + ...acc.currentGroup, + formatSyntaxTree( + pipe(withIndentation(horizontalPosition), withZeroDotInfo, withHorizontalPosition(horizontalPosition))( + config, + ), + )(arg), + ], + }; + }, + { groups: [], currentGroup: [] }, + ); + const argumentGroups = + greedyArgumentGroups.currentGroup.length > 0 + ? [...greedyArgumentGroups.groups, greedyArgumentGroups.currentGroup] + : greedyArgumentGroups.groups; + + const indentationWidth = horizontalPosition; + + const greedyFits = argumentGroups.every((group) => { + const groupWidth = group.map(({ width }) => width).reduce(sum, 0) + (group.length - 1) * 2; + return indentationWidth + groupWidth <= config.maxLineLength; + }); + + if (greedyFits) { + const lastArgumentGroup = last(argumentGroups); + // Add the width of the last line of parameters, the dots between them and the indentation of the parameters + const width = lastArgumentGroup + ? lastArgumentGroup.map(({ width }) => width).reduce(sum, 0) + (lastArgumentGroup.length - 1) * 2 + : 0; + return { + type: TokenType.Method, + method, + arguments: syntaxTree.arguments, + argumentGroups, + argumentsShouldStartOnNewLine: false, + localIndentation: config.localIndentation, + horizontalPosition: config.horizontalPosition, + shouldStartWithDot: false, + shouldEndWithDot: Boolean(config.shouldEndWithDot), + width, + }; + } + + // Fallback to wrap-all + const wrapAllArgumentGroups = syntaxTree.arguments.map((step) => [ formatSyntaxTree( - pipe(withIncreasedIndentation(2), withZeroDotInfo, withHorizontalPosition(config.localIndentation + 2))(config), + pipe(withIndentation(horizontalPosition), withZeroDotInfo, withHorizontalPosition(horizontalPosition))(config), )(step), ]); - const lastArgumentGroup = last(argumentGroups); - // Add the width of the last line of parameters, the dots between them and the indentation of the parameters - const width = lastArgumentGroup - ? lastArgumentGroup.map(({ width }) => width).reduce(sum, 0) + lastArgumentGroup.length - 1 + + const lastWrapAllArgumentGroup = last(wrapAllArgumentGroups); + const wrapAllWidth = lastWrapAllArgumentGroup + ? lastWrapAllArgumentGroup.map(({ width }) => width).reduce(sum, 0) + (lastWrapAllArgumentGroup.length - 1) * 2 : 0; + return { type: TokenType.Method, method, arguments: syntaxTree.arguments, - argumentGroups, - argumentsShouldStartOnNewLine: true, + argumentGroups: wrapAllArgumentGroups, + argumentsShouldStartOnNewLine: false, + localIndentation: config.localIndentation, + horizontalPosition: config.horizontalPosition, shouldStartWithDot: false, shouldEndWithDot: Boolean(config.shouldEndWithDot), - localIndentation: 0, - width, + width: wrapAllWidth, }; }; diff --git a/gremlint/src/formatQuery/formatSyntaxTrees/formatTraversal/getStepGroups/reduceSingleStepInStepGroup.ts b/gremlint/src/formatQuery/formatSyntaxTrees/formatTraversal/getStepGroups/reduceSingleStepInStepGroup.ts index 4b90fe7aa2a..7c43c868902 100644 --- a/gremlint/src/formatQuery/formatSyntaxTrees/formatTraversal/getStepGroups/reduceSingleStepInStepGroup.ts +++ b/gremlint/src/formatQuery/formatSyntaxTrees/formatTraversal/getStepGroups/reduceSingleStepInStepGroup.ts @@ -69,7 +69,7 @@ const reduceSingleStepInStepGroup = (formatSyntaxTree: GremlinSyntaxTreeFormatte pipe( withIndentation(localIndentation), withDotInfo({ shouldStartWithDot, shouldEndWithDot }), - withHorizontalPosition(localIndentation + +config.shouldPlaceDotsAfterLineBreaks), + withHorizontalPosition(localIndentation), )(config), )(step), ], diff --git a/gremlint/src/formatQuery/formatSyntaxTrees/formatTraversal/getStepGroups/utils.ts b/gremlint/src/formatQuery/formatSyntaxTrees/formatTraversal/getStepGroups/utils.ts index 13392fbe0c9..ba3c13b22ed 100644 --- a/gremlint/src/formatQuery/formatSyntaxTrees/formatTraversal/getStepGroups/utils.ts +++ b/gremlint/src/formatQuery/formatSyntaxTrees/formatTraversal/getStepGroups/utils.ts @@ -25,6 +25,7 @@ import { UnformattedSyntaxTree, } from '../../../types'; import { STEP_MODULATORS } from '../../../consts'; +import { willFitOnLine } from '../../../layoutUtils'; import recreateQueryOnelinerFromSyntaxTree from '../../../recreateQueryOnelinerFromSyntaxTree'; export const isTraversalSource = (step: UnformattedSyntaxTree | FormattedSyntaxTree): boolean => { @@ -78,15 +79,14 @@ const isLineTooLongWithSubsequentModulators = (config: GremlintInternalConfig) = return indentationIncrease; })(); - const recreatedQueryWithSubsequentModulators = recreateQueryOnelinerFromSyntaxTree( - config.localIndentation + stepGroupIndentationIncrease, - )({ - type: TokenType.Traversal, - steps: stepsWithSubsequentModulators, - }); - - const lineIsTooLongWithSubsequentModulators = recreatedQueryWithSubsequentModulators.length > config.maxLineLength; - return lineIsTooLongWithSubsequentModulators; + return !willFitOnLine( + { + type: TokenType.Traversal, + steps: stepsWithSubsequentModulators, + }, + config, + config.horizontalPosition + stepGroupIndentationIncrease, + ); }; // If the first step in a group is a modulator, then it must also be the last step in the group diff --git a/gremlint/src/formatQuery/formatSyntaxTrees/formatTraversal/index.ts b/gremlint/src/formatQuery/formatSyntaxTrees/formatTraversal/index.ts index 74417f4c2a8..108ac6653ff 100644 --- a/gremlint/src/formatQuery/formatSyntaxTrees/formatTraversal/index.ts +++ b/gremlint/src/formatQuery/formatSyntaxTrees/formatTraversal/index.ts @@ -27,6 +27,7 @@ import { UnformattedTraversalSyntaxTree, } from '../../types'; import { last, pipe, sum } from '../../utils'; +import { willFitOnLine } from '../../layoutUtils'; import { withIncreasedHorizontalPosition, withZeroIndentation } from '../utils'; import { getStepGroups } from './getStepGroups'; import { isTraversalSource } from './getStepGroups/utils'; @@ -37,10 +38,17 @@ export const formatTraversal = (formatSyntaxTree: GremlinSyntaxTreeFormatter) => ): FormattedTraversalSyntaxTree => { const initialHorizontalPositionIndentationIncrease = syntaxTree.steps[0] && isTraversalSource(syntaxTree.steps[0]) ? syntaxTree.initialHorizontalPosition : 0; - const recreatedQuery = recreateQueryOnelinerFromSyntaxTree( - config.localIndentation + initialHorizontalPositionIndentationIncrease, - )(syntaxTree); - if (recreatedQuery.length <= config.maxLineLength) { + if ( + willFitOnLine( + syntaxTree, + config, + config.horizontalPosition + initialHorizontalPositionIndentationIncrease, + ) + ) { + const recreatedQuery = recreateQueryOnelinerFromSyntaxTree( + config, + config.horizontalPosition + initialHorizontalPositionIndentationIncrease, + )(syntaxTree); return { type: TokenType.Traversal, steps: syntaxTree.steps, diff --git a/gremlint/src/formatQuery/index.ts b/gremlint/src/formatQuery/index.ts index 4f27e3e3746..174f89127ed 100644 --- a/gremlint/src/formatQuery/index.ts +++ b/gremlint/src/formatQuery/index.ts @@ -37,7 +37,7 @@ const getInternalGremlintConfig = ({ }: GremlintUserConfig): GremlintInternalConfig => ({ globalIndentation: indentation, localIndentation: 0, - maxLineLength: maxLineLength - indentation, + maxLineLength: maxLineLength, shouldPlaceDotsAfterLineBreaks, shouldStartWithDot: false, shouldEndWithDot: false, diff --git a/gremlint/src/formatQuery/layoutUtils.ts b/gremlint/src/formatQuery/layoutUtils.ts new file mode 100644 index 00000000000..311c48e0e5b --- /dev/null +++ b/gremlint/src/formatQuery/layoutUtils.ts @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import recreateQueryOnelinerFromSyntaxTree from './recreateQueryOnelinerFromSyntaxTree'; +import { GremlintInternalConfig, UnformattedSyntaxTree } from './types'; + +/** + * Checks if a syntax tree or a set of syntax trees can fit on a single line + * within the maxLineLength constraint. + */ +export const willFitOnLine = ( + syntaxTree: UnformattedSyntaxTree | { type: string; steps: UnformattedSyntaxTree[] }, + config: Pick, + horizontalPosition: number = 0, +): boolean => { + const recreatedQuery = recreateQueryOnelinerFromSyntaxTree({ globalIndentation: 0 }, horizontalPosition)( + syntaxTree as any, + ); + return config.globalIndentation + recreatedQuery.length <= config.maxLineLength; +}; + +/** + * Calculates the width of a syntax tree when formatted as a one-liner. + */ +export const getOneLinerWidth = ( + syntaxTree: UnformattedSyntaxTree, + horizontalPosition: number = 0, +): number => { + const recreatedQuery = recreateQueryOnelinerFromSyntaxTree({ globalIndentation: 0 }, horizontalPosition)( + syntaxTree as any, + ); + return recreatedQuery.trim().length; +}; diff --git a/gremlint/src/formatQuery/recreateQueryOnelinerFromSyntaxTree.ts b/gremlint/src/formatQuery/recreateQueryOnelinerFromSyntaxTree.ts index 9075d7e9734..db01ecde810 100644 --- a/gremlint/src/formatQuery/recreateQueryOnelinerFromSyntaxTree.ts +++ b/gremlint/src/formatQuery/recreateQueryOnelinerFromSyntaxTree.ts @@ -25,6 +25,7 @@ import { UnformattedStringSyntaxTree, UnformattedTraversalSyntaxTree, UnformattedWordSyntaxTree, + GremlintInternalConfig, } from './types'; import { last, spaces } from './utils'; @@ -36,27 +37,31 @@ type GremlinOnelinerSyntaxTree = | Pick | Pick; -const recreateQueryOnelinerFromSyntaxTree = (localIndentation: number = 0) => ( +const recreateQueryOnelinerFromSyntaxTree = (config: Pick, localIndentation: number = 0) => ( syntaxTree: GremlinOnelinerSyntaxTree, ): string => { + const indentation = spaces(config.globalIndentation + localIndentation); switch (syntaxTree.type) { // This case will never occur case TokenType.NonGremlinCode: return syntaxTree.code; case TokenType.Traversal: - return spaces(localIndentation) + syntaxTree.steps.map(recreateQueryOnelinerFromSyntaxTree()).join('.'); + return ( + indentation + + syntaxTree.steps.map(recreateQueryOnelinerFromSyntaxTree({ globalIndentation: 0 })).join('.') + ); case TokenType.Method: return ( - spaces(localIndentation) + - recreateQueryOnelinerFromSyntaxTree()(syntaxTree.method) + + indentation + + recreateQueryOnelinerFromSyntaxTree({ globalIndentation: 0 })(syntaxTree.method) + '(' + - syntaxTree.arguments.map(recreateQueryOnelinerFromSyntaxTree()).join(', ') + + syntaxTree.arguments.map(recreateQueryOnelinerFromSyntaxTree({ globalIndentation: 0 })).join(', ') + ')' ); case TokenType.Closure: return ( - spaces(localIndentation) + - recreateQueryOnelinerFromSyntaxTree()(syntaxTree.method) + + indentation + + recreateQueryOnelinerFromSyntaxTree({ globalIndentation: 0 })(syntaxTree.method) + '{' + last( syntaxTree.closureCodeBlock.map( @@ -66,9 +71,9 @@ const recreateQueryOnelinerFromSyntaxTree = (localIndentation: number = 0) => ( '}' ); case TokenType.String: - return spaces(localIndentation) + syntaxTree.string; + return indentation + syntaxTree.string; case TokenType.Word: - return spaces(localIndentation) + syntaxTree.word; + return indentation + syntaxTree.word; } }; diff --git a/gremlint/src/formatQuery/recreateQueryStringFromFormattedSyntaxTrees.ts b/gremlint/src/formatQuery/recreateQueryStringFromFormattedSyntaxTrees.ts index e7d9f6af613..2fa64b0dbf1 100644 --- a/gremlint/src/formatQuery/recreateQueryStringFromFormattedSyntaxTrees.ts +++ b/gremlint/src/formatQuery/recreateQueryStringFromFormattedSyntaxTrees.ts @@ -26,20 +26,26 @@ const recreateQueryStringFromFormattedSyntaxTree = (syntaxTree: FormattedSyntaxT } if (syntaxTree.type === TokenType.Traversal) { return syntaxTree.stepGroups - .map((stepGroup) => stepGroup.steps.map(recreateQueryStringFromFormattedSyntaxTree).join('.')) + .map((stepGroup, index) => { + const firstStep = stepGroup.steps[0]; + const indentationAmount = + index === 0 || firstStep.type === TokenType.NonGremlinCode ? 0 : firstStep.localIndentation; + const indentation = spaces(indentationAmount); + return indentation + stepGroup.steps.map(recreateQueryStringFromFormattedSyntaxTree).join('.'); + }) .join('\n'); } if (syntaxTree.type === TokenType.Method) { + const methodOpeningParenthesis = + (syntaxTree.shouldStartWithDot ? '.' : '') + recreateQueryStringFromFormattedSyntaxTree(syntaxTree.method) + '('; + const indentation = spaces(syntaxTree.horizontalPosition + methodOpeningParenthesis.length); return ( - (syntaxTree.shouldStartWithDot ? '.' : '') + - [ - recreateQueryStringFromFormattedSyntaxTree(syntaxTree.method) + '(', - syntaxTree.argumentGroups - .map((args) => args.map(recreateQueryStringFromFormattedSyntaxTree).join(', ')) - .join(',\n') + - ')' + - (syntaxTree.shouldEndWithDot ? '.' : ''), - ].join(syntaxTree.argumentsShouldStartOnNewLine ? '\n' : '') + methodOpeningParenthesis + + syntaxTree.argumentGroups + .map((args) => args.map(recreateQueryStringFromFormattedSyntaxTree).join(', ')) + .join(',\n' + indentation) + + ')' + + (syntaxTree.shouldEndWithDot ? '.' : '') ); } if (syntaxTree.type === TokenType.Closure) { @@ -55,15 +61,10 @@ const recreateQueryStringFromFormattedSyntaxTree = (syntaxTree: FormattedSyntaxT ); } if (syntaxTree.type === TokenType.String) { - return spaces(syntaxTree.localIndentation) + syntaxTree.string; + return syntaxTree.string; } if (syntaxTree.type === TokenType.Word) { - return ( - spaces(syntaxTree.localIndentation) + - (syntaxTree.shouldStartWithDot ? '.' : '') + - syntaxTree.word + - (syntaxTree.shouldEndWithDot ? '.' : '') - ); + return (syntaxTree.shouldStartWithDot ? '.' : '') + syntaxTree.word + (syntaxTree.shouldEndWithDot ? '.' : ''); } // The following line is just here to convince TypeScript that the return type // is string and not string | undefined. diff --git a/gremlint/src/formatQuery/types.ts b/gremlint/src/formatQuery/types.ts index f85e91a166d..45999c62610 100644 --- a/gremlint/src/formatQuery/types.ts +++ b/gremlint/src/formatQuery/types.ts @@ -117,6 +117,7 @@ export type FormattedMethodSyntaxTree = { argumentGroups: FormattedSyntaxTree[][]; argumentsShouldStartOnNewLine: boolean; localIndentation: number; + horizontalPosition: number; width: number; shouldStartWithDot: boolean; shouldEndWithDot: boolean;