midgard/mapcomplete
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Add code quality test, remove all constructor.name entries as they are unreadable after minification

Browse source
This commit is contained in:
pietervdvn 2022-02-14 02:26:03 +01:00
parent 312dbe7aff
commit 1c418e5a49
15 changed files with 95 additions and 52 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

22
Logic/DetermineLayout.ts
View file

@ -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<string>
): 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(
`<a href="${link}">${link}</a> is invalid:`,
new FixedUiElement(e)
new FixedUiElement(e),
parsed
)
return null;
}

1
Logic/ExtraFunctions.ts
View file

@ -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))

2
Logic/ImageProviders/ImageProvider.ts
View file

@ -35,7 +35,7 @@ export default abstract class ImageProvider {
}): UIEventSource<ProvidedImage[]> {
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<string>()

2
Logic/Osm/Actions/OsmChangeAction.ts
View file

@ -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)

15
Models/ThemeConfig/Conversion/Conversion.ts
View file

@ -12,10 +12,10 @@ export abstract class Conversion<TIn, TOut> {
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<T>(fixed: { errors?: string[], warnings?: string[], information?: string[], result?: T }): T {
@ -94,8 +94,8 @@ export class OnEveryConcat<X, T> extends DesugaringStep<T> {
private readonly step: Conversion<X, X[]>;
constructor(key: string, step: Conversion<X, X[]>) {
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<T> extends DesugaringStep<T> {
private readonly steps: DesugaringStep<T>[];
constructor(doc: string, ...steps: DesugaringStep<T>[]) {
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<T> extends DesugaringStep<T> {
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;

2
Models/ThemeConfig/Conversion/CreateNoteImportLayer.ts
View file

@ -16,7 +16,7 @@ export default class CreateNoteImportLayer extends Conversion<LayerConfigJson, L
super([
"Advanced conversion which deducts a layer showing all notes that are 'importable' (i.e. a note that contains a link to some MapComplete theme, with hash '#import').",
"The import buttons and matches will be based on the presets of the given theme",
].join("\n\n"), [])
].join("\n\n"), [],"CreateNoteImportLayer")
this._includeClosedNotesDays = includeClosedNotesDays;
}

4
Models/ThemeConfig/Conversion/FixImages.ts
View file

@ -6,7 +6,7 @@ import * as tagrenderingmetapaths from "../../../assets/tagrenderingconfigmeta.j
export class ExtractImages extends Conversion<LayoutConfigJson, string[]> {
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<LayoutConfigJson> {
private readonly _knownImages: Set<string>;
constructor(knownImages: Set<string>) {
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;
}

5
Models/ThemeConfig/Conversion/LegacyJsonConvert.ts
View file

@ -8,7 +8,8 @@ export class UpdateLegacyLayer extends DesugaringStep<LayerConfigJson | string |
constructor() {
super("Updates various attributes from the old data format to the new to provide backwards compatibility with the formats",
["overpassTags", "source.osmtags", "tagRenderings[*].id", "mapRendering"]);
["overpassTags", "source.osmtags", "tagRenderings[*].id", "mapRendering"],
"UpdateLegacyLayer");
}
convert(json: LayerConfigJson, context: string): { result: LayerConfigJson; errors: string[]; warnings: string[] } {
@ -120,7 +121,7 @@ export class UpdateLegacyLayer extends DesugaringStep<LayerConfigJson | string |
class UpdateLegacyTheme extends DesugaringStep<LayoutConfigJson> {
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[] } {

5
Models/ThemeConfig/Conversion/PrepareLayer.ts
View file

@ -8,7 +8,7 @@ import {Translation} from "../../../UI/i18n/Translation";
class ExpandTagRendering extends Conversion<string | TagRenderingConfigJson | { builtin: string | string[], override: any }, TagRenderingConfigJson[]> {
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)
}

12
Models/ThemeConfig/Conversion/PrepareTheme.ts
View file

@ -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<LayoutConfigJson> {
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<LayoutConfigJson> {
class AddImportLayers extends DesugaringStep<LayoutConfigJson> {
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<LayoutConfigJson> {
export class AddMiniMap extends DesugaringStep<LayerConfigJson> {
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<LayerConfigJson> {
class ApplyOverrideAll extends DesugaringStep<LayoutConfigJson> {
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<LayoutConfigJson> {
class AddDependencyLayersToTheme extends DesugaringStep<LayoutConfigJson> {
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;
}

12
Models/ThemeConfig/Conversion/Validation.ts
View file

@ -16,7 +16,7 @@ class ValidateLanguageCompleteness extends DesugaringStep<any> {
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<any> {
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<LayoutConfigJson> {
private readonly _isBuiltin: boolean;
constructor(knownImagePaths: Set<string>, 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<LayoutConfigJson> {
class OverrideShadowingCheck extends DesugaringStep<LayoutConfigJson> {
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<LayoutConfigJson> {
export class DetectShadowedMappings extends DesugaringStep<TagRenderingConfigJson> {
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<LayerConfigJson> {
private readonly _isBuiltin: boolean;
constructor(knownImagePaths: Set<string>, 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;

7
UI/BaseUIElement.ts
View file

@ -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() {

8
scripts/generateCache.ts
View file

@ -458,11 +458,13 @@ export async function main(args: string[]) {
let args = [...process.argv]
args.splice(0, 2)
try {
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) {
} catch (e) {
console.error("Error building cache:", e)
}
}

36
test/CodeQuality.spec.ts Normal file
View file

@ -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."
}
}))
}
]
]);
}
}

4
test/TestAll.ts
View file

@ -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;