diff --git a/Logic/DetermineLayout.ts b/Logic/DetermineLayout.ts index 744198296..c58012c69 100644 --- a/Logic/DetermineLayout.ts +++ b/Logic/DetermineLayout.ts @@ -19,6 +19,7 @@ import {PrepareTheme} from "../Models/ThemeConfig/Conversion/PrepareTheme"; import * as licenses from "../assets/generated/license_info.json" import TagRenderingConfig from "../Models/ThemeConfig/TagRenderingConfig"; import {FixImages} from "../Models/ThemeConfig/Conversion/FixImages"; +import Svg from "../Svg"; export default class DetermineLayout { @@ -73,6 +74,8 @@ export default class DetermineLayout { userLayoutParam: UIEventSource ): LayoutConfig | null { let hash = location.hash.substr(1); + let json: any; + try { // layoutFromBase64 contains the name of the theme. This is partly to do tracking with goat counter const dedicatedHashFromLocalStorage = LocalStorageSource.Get( @@ -95,7 +98,6 @@ export default class DetermineLayout { dedicatedHashFromLocalStorage.setData(hash); } - let json: any; try { json = JSON.parse(atob(hash)); } catch (e) { @@ -115,23 +117,26 @@ export default class DetermineLayout { } catch (e) { console.error(e) if (hash === undefined || hash.length < 10) { - DetermineLayout.ShowErrorOnCustomTheme("Could not load a theme from the hash", new FixedUiElement("Hash does not contain data")) + DetermineLayout.ShowErrorOnCustomTheme("Could not load a theme from the hash", new FixedUiElement("Hash does not contain data"), json) } - this.ShowErrorOnCustomTheme("Could not parse the hash", new FixedUiElement(e)) + this.ShowErrorOnCustomTheme("Could not parse the hash", new FixedUiElement(e), json) return null; } } public static ShowErrorOnCustomTheme( intro: string = "Error: could not parse the custom layout:", - error: BaseUIElement) { + error: BaseUIElement, + json?: any) { new Combine([ intro, error.SetClass("alert"), - new SubtleButton("./assets/svg/mapcomplete_logo.svg", + new SubtleButton(Svg.back_svg(), "Go back to the theme overview", - {url: window.location.protocol + "//" + window.location.hostname + "/index.html", newTab: false}) - + {url: window.location.protocol + "//" + window.location.hostname + "/index.html", newTab: false}), + json !== undefined ? new SubtleButton(Svg.download_svg(),"Download the JSON file").onClick(() => { + Utils.offerContentsAsDownloadableFile(JSON.stringify(json, null, " "), "theme_definition.json") + }) : undefined ]) .SetClass("flex flex-col clickable") .AttachTo("centermessage"); @@ -189,7 +194,8 @@ export default class DetermineLayout { console.error(e) DetermineLayout.ShowErrorOnCustomTheme( `${link} is invalid:`, - new FixedUiElement(e) + new FixedUiElement(e), + parsed ) return null; } diff --git a/Logic/ExtraFunctions.ts b/Logic/ExtraFunctions.ts index 242e323e8..9a97ff39e 100644 --- a/Logic/ExtraFunctions.ts +++ b/Logic/ExtraFunctions.ts @@ -413,7 +413,6 @@ export class ExtraFunctions { const elems = [] for (const func of ExtraFunctions.allFuncs) { - console.log("Generating ", func.constructor.name) elems.push(new Title(func._name, 3), func._doc, new List(func._args ?? [], true)) diff --git a/Logic/ImageProviders/ImageProvider.ts b/Logic/ImageProviders/ImageProvider.ts index c1d90578e..dded72a0b 100644 --- a/Logic/ImageProviders/ImageProvider.ts +++ b/Logic/ImageProviders/ImageProvider.ts @@ -35,7 +35,7 @@ export default abstract class ImageProvider { }): UIEventSource { const prefixes = options?.prefixes ?? this.defaultKeyPrefixes if (prefixes === undefined) { - throw "The image provider" + this.constructor.name + " doesn't define `defaultKeyPrefixes`" + throw "No `defaultKeyPrefixes` defined by this image provider" } const relevantUrls = new UIEventSource<{ url: string; key: string; provider: ImageProvider }[]>([]) const seenValues = new Set() diff --git a/Logic/Osm/Actions/OsmChangeAction.ts b/Logic/Osm/Actions/OsmChangeAction.ts index 120870f73..1754a91bc 100644 --- a/Logic/Osm/Actions/OsmChangeAction.ts +++ b/Logic/Osm/Actions/OsmChangeAction.ts @@ -23,7 +23,7 @@ export default abstract class OsmChangeAction { public Perform(changes: Changes) { if (this.isUsed) { - throw "This ChangeAction is already used: " + this.constructor.name + throw "This ChangeAction is already used" } this.isUsed = true; return this.CreateChangeDescriptions(changes) diff --git a/Models/ThemeConfig/Conversion/Conversion.ts b/Models/ThemeConfig/Conversion/Conversion.ts index d90bfca0b..2926fb3de 100644 --- a/Models/ThemeConfig/Conversion/Conversion.ts +++ b/Models/ThemeConfig/Conversion/Conversion.ts @@ -12,10 +12,10 @@ export abstract class Conversion { protected readonly doc: string; public readonly name: string - constructor(doc: string, modifiedAttributes: string[] = [], name?: string) { + constructor(doc: string, modifiedAttributes: string[] = [], name: string) { this.modifiedAttributes = modifiedAttributes; this.doc = doc + "\n\nModified attributes are\n" + modifiedAttributes.join(", "); - this.name = name ?? this.constructor.name + this.name = name } public static strict(fixed: { errors?: string[], warnings?: string[], information?: string[], result?: T }): T { @@ -94,8 +94,8 @@ export class OnEveryConcat extends DesugaringStep { private readonly step: Conversion; constructor(key: string, step: Conversion) { - super(`Applies ${step.constructor.name} onto every object of the list \`${key}\`. The results are concatenated and used as new list`, [key], - "OnEveryConcat("+step.name+")"); + super(`Applies ${step.name} onto every object of the list \`${key}\`. The results are concatenated and used as new list`, [key], + "OnEvery("+key+").Concat("+step.name+")"); this.step = step; this.key = key; } @@ -126,8 +126,9 @@ export class Fuse extends DesugaringStep { private readonly steps: DesugaringStep[]; constructor(doc: string, ...steps: DesugaringStep[]) { - super((doc ?? "") + "This fused pipeline of the following steps: " + steps.map(s => s.constructor.name).join(", "), - Utils.Dedup([].concat(...steps.map(step => step.modifiedAttributes))) + super((doc ?? "") + "This fused pipeline of the following steps: " + steps.map(s => s.name).join(", "), + Utils.Dedup([].concat(...steps.map(step => step.modifiedAttributes))), + "Fuse of "+steps.map(s => s.name).join(", ") ); this.steps = steps; } @@ -163,7 +164,7 @@ export class SetDefault extends DesugaringStep { private readonly _overrideEmptyString: boolean; constructor(key: string, value: any, overrideEmptyString = false) { - super("Sets " + key + " to a default value if undefined"); + super("Sets " + key + " to a default value if undefined", [], "SetDefault of "+key); this.key = key; this.value = value; this._overrideEmptyString = overrideEmptyString; diff --git a/Models/ThemeConfig/Conversion/CreateNoteImportLayer.ts b/Models/ThemeConfig/Conversion/CreateNoteImportLayer.ts index c51437d7f..49d7af5b9 100644 --- a/Models/ThemeConfig/Conversion/CreateNoteImportLayer.ts +++ b/Models/ThemeConfig/Conversion/CreateNoteImportLayer.ts @@ -16,7 +16,7 @@ export default class CreateNoteImportLayer extends Conversion { constructor() { - super("Extract all images from a layoutConfig using the meta paths"); + super("Extract all images from a layoutConfig using the meta paths",[],"ExctractImages"); } convert(json: LayoutConfigJson, context: string): { result: string[], errors: string[] } { @@ -62,7 +62,7 @@ export class FixImages extends DesugaringStep { private readonly _knownImages: Set; constructor(knownImages: Set) { - super("Walks over the entire theme and replaces images to the relative URL. Only works if the ID of the theme is an URL"); + super("Walks over the entire theme and replaces images to the relative URL. Only works if the ID of the theme is an URL",[],"fixImages"); this._knownImages = knownImages; } diff --git a/Models/ThemeConfig/Conversion/LegacyJsonConvert.ts b/Models/ThemeConfig/Conversion/LegacyJsonConvert.ts index cf89a5172..f5a5294fb 100644 --- a/Models/ThemeConfig/Conversion/LegacyJsonConvert.ts +++ b/Models/ThemeConfig/Conversion/LegacyJsonConvert.ts @@ -8,7 +8,8 @@ export class UpdateLegacyLayer extends DesugaringStep { constructor() { - super("Small fixes in the theme config", ["roamingRenderings"]); + super("Small fixes in the theme config", ["roamingRenderings"],"UpdateLegacyTheme"); } convert(json: LayoutConfigJson, context: string): { result: LayoutConfigJson; errors: string[]; warnings: string[] } { diff --git a/Models/ThemeConfig/Conversion/PrepareLayer.ts b/Models/ThemeConfig/Conversion/PrepareLayer.ts index 94b664df0..673e131e8 100644 --- a/Models/ThemeConfig/Conversion/PrepareLayer.ts +++ b/Models/ThemeConfig/Conversion/PrepareLayer.ts @@ -8,7 +8,7 @@ import {Translation} from "../../../UI/i18n/Translation"; class ExpandTagRendering extends Conversion { private readonly _state: DesugaringContext; constructor(state: DesugaringContext) { - super("Converts a tagRenderingSpec into the full tagRendering", []); + super("Converts a tagRenderingSpec into the full tagRendering", [],"ExpandTagRendering"); this._state = state; } @@ -147,7 +147,8 @@ class ExpandGroupRewrite extends Conversion<{ constructor(state: DesugaringContext) { super( - "Converts a rewrite config for tagRenderings into the expanded form" + "Converts a rewrite config for tagRenderings into the expanded form",[], + "ExpandGroupRewrite" ); this._expandSubTagRenderings = new ExpandTagRendering(state) } diff --git a/Models/ThemeConfig/Conversion/PrepareTheme.ts b/Models/ThemeConfig/Conversion/PrepareTheme.ts index f2270c014..17e04ac5a 100644 --- a/Models/ThemeConfig/Conversion/PrepareTheme.ts +++ b/Models/ThemeConfig/Conversion/PrepareTheme.ts @@ -16,7 +16,7 @@ class SubstituteLayer extends Conversion<(string | LayerConfigJson), LayerConfig constructor( state: DesugaringContext, ) { - super("Converts the identifier of a builtin layer into the actual layer, or converts a 'builtin' syntax with override in the fully expanded form", []); + super("Converts the identifier of a builtin layer into the actual layer, or converts a 'builtin' syntax with override in the fully expanded form", [],"SubstuteLayers"); this._state = state; } @@ -130,7 +130,7 @@ class AddDefaultLayers extends DesugaringStep { private _state: DesugaringContext; constructor(state: DesugaringContext) { - super("Adds the default layers, namely: " + Constants.added_by_default.join(", "), ["layers"]); + super("Adds the default layers, namely: " + Constants.added_by_default.join(", "), ["layers"],"AddDefaultLayers"); this._state = state; } @@ -182,7 +182,7 @@ class AddDefaultLayers extends DesugaringStep { class AddImportLayers extends DesugaringStep { constructor() { - super("For every layer in the 'layers'-list, create a new layer which'll import notes. (Note that priviliged layers and layers which have a geojson-source set are ignored)", ["layers"]); + super("For every layer in the 'layers'-list, create a new layer which'll import notes. (Note that priviliged layers and layers which have a geojson-source set are ignored)", ["layers"],"AddImportLayers"); } convert(json: LayoutConfigJson, context: string): { result: LayoutConfigJson; errors: string[] } { @@ -240,7 +240,7 @@ class AddImportLayers extends DesugaringStep { export class AddMiniMap extends DesugaringStep { private readonly _state: DesugaringContext; constructor(state: DesugaringContext, ) { - super("Adds a default 'minimap'-element to the tagrenderings if none of the elements define such a minimap", ["tagRenderings"]); + super("Adds a default 'minimap'-element to the tagrenderings if none of the elements define such a minimap", ["tagRenderings"],"AddMiniMap"); this._state = state; } @@ -291,7 +291,7 @@ export class AddMiniMap extends DesugaringStep { class ApplyOverrideAll extends DesugaringStep { constructor() { - super("Applies 'overrideAll' onto every 'layer'. The 'overrideAll'-field is removed afterwards", ["overrideAll", "layers"]); + super("Applies 'overrideAll' onto every 'layer'. The 'overrideAll'-field is removed afterwards", ["overrideAll", "layers"],"ApplyOverrideAll"); } convert(json: LayoutConfigJson, context: string): { result: LayoutConfigJson; errors: string[]; warnings: string[] } { @@ -321,7 +321,7 @@ class ApplyOverrideAll extends DesugaringStep { class AddDependencyLayersToTheme extends DesugaringStep { private readonly _state: DesugaringContext; constructor(state: DesugaringContext, ) { - super("If a layer has a dependency on another layer, these layers are added automatically on the theme. (For example: defibrillator depends on 'walls_and_buildings' to snap onto. This layer is added automatically)", ["layers"]); + super("If a layer has a dependency on another layer, these layers are added automatically on the theme. (For example: defibrillator depends on 'walls_and_buildings' to snap onto. This layer is added automatically)", ["layers"],"AddDependencyLayersToTheme"); this._state = state; } diff --git a/Models/ThemeConfig/Conversion/Validation.ts b/Models/ThemeConfig/Conversion/Validation.ts index 777e9c614..c28886088 100644 --- a/Models/ThemeConfig/Conversion/Validation.ts +++ b/Models/ThemeConfig/Conversion/Validation.ts @@ -16,7 +16,7 @@ class ValidateLanguageCompleteness extends DesugaringStep { private readonly _languages: string[]; constructor(...languages: string[]) { - super("Checks that the given object is fully translated in the specified languages", []); + super("Checks that the given object is fully translated in the specified languages", [], "ValidateLanguageCompleteness"); this._languages = languages; } @@ -25,7 +25,7 @@ class ValidateLanguageCompleteness extends DesugaringStep { const translations = Translation.ExtractAllTranslationsFrom( obj ) - for (const neededLanguage of this._languages) { + for (const neededLanguage of this._languages ?? ["en"]) { translations .filter(t => t.tr.translations[neededLanguage] === undefined && t.tr.translations["*"] === undefined) .forEach(missing => { @@ -50,7 +50,7 @@ class ValidateTheme extends DesugaringStep { private readonly _isBuiltin: boolean; constructor(knownImagePaths: Set, path: string, isBuiltin: boolean) { - super("Doesn't change anything, but emits warnings and errors", []); + super("Doesn't change anything, but emits warnings and errors", [],"ValidateTheme"); this.knownImagePaths = knownImagePaths; this._path = path; this._isBuiltin = isBuiltin; @@ -164,7 +164,7 @@ export class ValidateThemeAndLayers extends Fuse { class OverrideShadowingCheck extends DesugaringStep { constructor() { - super("Checks that an 'overrideAll' does not override a single override"); + super("Checks that an 'overrideAll' does not override a single override",[],"OverrideShadowingCheck"); } convert(json: LayoutConfigJson, context: string): { result: LayoutConfigJson; errors?: string[]; warnings?: string[] } { @@ -204,7 +204,7 @@ export class PrevalidateTheme extends Fuse { export class DetectShadowedMappings extends DesugaringStep { constructor() { - super("Checks that the mappings don't shadow each other"); + super("Checks that the mappings don't shadow each other",[],"DetectShadowedMappings"); } convert(json: TagRenderingConfigJson, context: string): { result: TagRenderingConfigJson; errors?: string[]; warnings?: string[] } { @@ -254,7 +254,7 @@ export class ValidateLayer extends DesugaringStep { private readonly _isBuiltin: boolean; constructor(knownImagePaths: Set, path: string, isBuiltin: boolean) { - super("Doesn't change anything, but emits warnings and errors", []); + super("Doesn't change anything, but emits warnings and errors", [],"ValidateLayer"); this.knownImagePaths = knownImagePaths; this._path = path; this._isBuiltin = isBuiltin; diff --git a/UI/BaseUIElement.ts b/UI/BaseUIElement.ts index 61d55c251..2157fb2f1 100644 --- a/UI/BaseUIElement.ts +++ b/UI/BaseUIElement.ts @@ -102,11 +102,6 @@ export default abstract class BaseUIElement { return this._constructedHtmlElement } - if (this.InnerConstructElement === undefined) { - throw "ERROR! This is not a correct baseUIElement: " + this.constructor.name - } - - try { const el = this.InnerConstructElement(); @@ -152,7 +147,7 @@ export default abstract class BaseUIElement { } public AsMarkdown(): string { - throw "AsMarkdown is not implemented by " + this.constructor.name + "; implement it in the subclass" + throw "AsMarkdown is not implemented; implement it in the subclass" } public Destroy() { diff --git a/scripts/generateCache.ts b/scripts/generateCache.ts index da36e76df..36f3303ce 100644 --- a/scripts/generateCache.ts +++ b/scripts/generateCache.ts @@ -458,11 +458,13 @@ export async function main(args: string[]) { let args = [...process.argv] -args.splice(0, 2) -try { - main(args) - .then(() => console.log("All done!")) - .catch(e => console.error("Error building cache:", e)); -} catch (e) { - console.error("Error building cache:", e) -} \ No newline at end of file +if (!args[1]?.endsWith("test/TestAll.ts")) { + args.splice(0, 2) + try { + main(args) + .then(() => console.log("All done!")) + .catch(e => console.error("Error building cache:", e)); + } catch (e) { + console.error("Error building cache:", e) + } +} diff --git a/test/CodeQuality.spec.ts b/test/CodeQuality.spec.ts new file mode 100644 index 000000000..53fe24587 --- /dev/null +++ b/test/CodeQuality.spec.ts @@ -0,0 +1,36 @@ +import T from "./TestHelper"; +import {exec} from "child_process"; + +export default class CodeQualitySpec extends T { + constructor() { + super([ + [ + "no constructor.name in compiled code", () => { + + const excludedDirs = [".git", "node_modules", "dist", ".cache", ".parcel-cache", "assets"] + + exec("grep \"constructor.name\" -r . " + excludedDirs.map(d => "--exclude-dir=" + d).join(" "), ((error, stdout, stderr) => { + console.log("Grep gave: ", stdout) + if (error?.message?.startsWith("Command failed: grep")) { + return; + } + if (error !== null) { + throw error + + } + if (stderr !== "") { + throw stderr + } + + const found = stdout.split("\n").filter(s => s !== "").filter(s => s.startsWith("test/")); + if (found.length > 0) { + throw "Found a 'constructor.name' at " + found.join(", ") + ". This is not allowed, as minification does erase names." + } + + })) + + } + ] + ]); + } +} \ No newline at end of file diff --git a/test/TestAll.ts b/test/TestAll.ts index 8251c4fab..28cf24b54 100644 --- a/test/TestAll.ts +++ b/test/TestAll.ts @@ -18,6 +18,7 @@ import LegacyThemeLoaderSpec from "./LegacyThemeLoader.spec"; import T from "./TestHelper"; import CreateNoteImportLayerSpec from "./CreateNoteImportLayer.spec"; import CreateCacheSpec from "./CreateCache.spec"; +import CodeQualitySpec from "./CodeQuality.spec"; async function main() { @@ -39,7 +40,8 @@ async function main() { new ReplaceGeometrySpec(), new LegacyThemeLoaderSpec(), new CreateNoteImportLayerSpec(), - new CreateCacheSpec() + new CreateCacheSpec(), + new CodeQualitySpec() ] ScriptUtils.fixUtils(); const realDownloadFunc = Utils.externalDownloadFunction;