\${{rendered_value}}
+ {% elsif bat._value == "eur" %} +€{{rendered_value}}
+ {% else %} +{{rendered_value}}
+ {% endif %} ;; + } + } + }`)); + expect(result).toContainMessage(summary(1, 0, 0)); + expect(result).toContainMessage({...F1}); + }); + + it('should error for 3-part cross-view special-suffix with multiple references in html {% %}', () => { + let result = rule(parse(`model: my_model { + view: foo { + sql_table_name: foo ;; + dimension: bar { + html: {% if bat._value == "usd" or baz.bat._value == "cad" %} +\${{rendered_value}}
+ {% elsif bat._value == "eur" %} +€{{rendered_value}}
+ {% else %} +{{rendered_value}}
+ {% endif %} ;; + } + } + }`)); + expect(result).toContainMessage(summary(1, 0, 1)); + expect(result).toContainMessage({...F1, ...error}); + }); + // TODO: Expose assembleModels from parser to test extensions here without full parseFiles & dummy project test case // it('should not error for a view with extension:required', () => { // let result = rule(parse(`model: my_model { diff --git a/docs/release-notes/v3.md b/docs/release-notes/v3.md index 8653de6..5600226 100644 --- a/docs/release-notes/v3.md +++ b/docs/release-notes/v3.md @@ -76,3 +76,4 @@ Patches: - v3.1.2 - Fix license metadata to consistently reflect MIT license - Update vulnerable dependencies (both in root package and tools/rule-sandbox) + - v3.1.3 - Fix rule F1 logic to check all references when there are multiple in a field ([Issue 167](https://github.com/looker-open-source/look-at-me-sideways/issues/167) [PR 168](https://github.com/looker-open-source/look-at-me-sideways/pull/168)) diff --git a/npm-shrinkwrap.dev.json b/npm-shrinkwrap.dev.json index 5ce02a4..46194ed 100644 --- a/npm-shrinkwrap.dev.json +++ b/npm-shrinkwrap.dev.json @@ -1,12 +1,12 @@ { "name": "@looker/look-at-me-sideways", - "version": "3.1.2", + "version": "3.1.3", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@looker/look-at-me-sideways", - "version": "3.1.2", + "version": "3.1.3", "license": "MIT", "dependencies": { "dot": "^1.1.3", diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 5ce02a4..46194ed 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -1,12 +1,12 @@ { "name": "@looker/look-at-me-sideways", - "version": "3.1.2", + "version": "3.1.3", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@looker/look-at-me-sideways", - "version": "3.1.2", + "version": "3.1.3", "license": "MIT", "dependencies": { "dot": "^1.1.3", diff --git a/package.json b/package.json index 70fe93c..7174b4a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@looker/look-at-me-sideways", - "version": "3.1.2", + "version": "3.1.3", "description": "A judgmental little LookML linter >_>", "main": "index.js", "directories": { diff --git a/rules/f1.js b/rules/f1.js index eaaa547..d2b1869 100644 --- a/rules/f1.js +++ b/rules/f1.js @@ -43,38 +43,57 @@ function ruleFn(match, path, project) { if (!referenceContainer || !referenceContainer.replace) { continue; } - // TODO: Currently only checking the first reference in each block/container, should loop through them all - let regexMatch = referenceContainer - .replace(/\b\d+\.\d+\b/g, '') // Remove decimals - .match(/(\$\{|\{\{|\{%)\s*(([^.{}]+)(\.[^.{}]+)+)\s*(%\}|\})/); - let parts = ((regexMatch || [])[2] || '').split('.').map((p)=>p.trim()).filter(Boolean); - if (!parts.length) { - continue; - } - // Don't treat references to TABLE or to own default alias as cross-view - if (parts[0] === 'TABLE' || parts[0] === view.$name) { - parts.shift(); + + referenceContainer = referenceContainer.replace(/\b\d+\.\d+\b/g, ''); // Remove decimals + + let regexPattern = /(\$\{|\{\{|\{%)\s*(([^.{}]+)(\.[^.{}]+)+)\s*(%\}|\})/g; + + let matches = []; + let match; + while ((match = regexPattern.exec(referenceContainer)) !== null) { + matches.push(...match[2].trim().split(' ').filter((str) => str.includes('.')).filter(Boolean)); } - // Don't treat references to special properties as cross-view - // Note: view._in_query,_is_filtered,_is_selected should not be allowed in fields - if ([ - 'SQL_TABLE_NAME', - '_sql', - '_value', - '_name', - '_filters', - '_parameter_value', - '_rendered_value', - '_label', - '_link', - ].includes(parts[parts.length - 1]) - ) { - parts.pop(); + + let fieldPaths = [...new Set(matches)]; // remove duplicates + + if (!fieldPaths.length) { + continue; } - if (parts.length > 1) { + + // find all of the field paths that uses cross view references + let crossViewFieldPaths = fieldPaths.filter((fieldPath) => { + let parts = fieldPath.split('.'); + + // Don't treat references to TABLE or to own default alias as cross-view + if (parts[0] === 'TABLE' || parts[0] === view.$name) { + return false; + } + + // Don't treat references to special properties as cross-view + // Note: view._in_query,_is_filtered,_is_selected should not be allowed in fields + if (parts.length == 2 && [ // only matches the fields that have two parts + 'SQL_TABLE_NAME', + '_sql', + '_value', + '_name', + '_filters', + '_parameter_value', + '_rendered_value', + '_label', + '_link', + ].includes(parts[parts.length - 1]) + ) { + return false; + } + return true; + }); + + if (crossViewFieldPaths.length > 0) { + // only report the first cross-view reference error + let parts = crossViewFieldPaths[0].split('.'); messages.push({ level: 'error', - description: `${field.$name} references another view, ${parts[0]}, via ${parts.join('.')}`, + description: `${field.$name} references another view, ${parts[0]}, via ${crossViewFieldPaths[0]}`, }); } }